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/04/26 03:57:14 UTC
[GitHub] [ratis] GlenGeng opened a new pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
GlenGeng opened a new pull request #473:
URL: https://github.com/apache/ratis/pull/473
## What changes were proposed in this pull request?
For now, if there is no snapshot existed, the returned snapshotIndex is 0. It is not correctly, since the raft log starts from index 0, a snapshot taken at snapshotIndex 0 should contain the log entry 0. The snapshotIndex for nn empty/fake snapshot should be -1.
```
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
index 728f7e9c..6307ca79 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
@@ -431,7 +431,7 @@ class ServerState implements Closeable {
long getLatestInstalledSnapshotIndex() {
final TermIndex ti = latestInstalledSnapshot.get();
- return ti != null? ti.getIndex(): 0L;
+ return ti != null? ti.getIndex(): -1L;
}
/**
@@ -440,7 +440,7 @@ class ServerState implements Closeable {
*/
long getSnapshotIndex() {
final SnapshotInfo s = getLatestSnapshot();
- final long latestSnapshotIndex = s != null ? s.getIndex() : 0;
+ final long latestSnapshotIndex = s != null ? s.getIndex() : -1;
return Math.max(latestSnapshotIndex, getLatestInstalledSnapshotIndex());
}
```
This issue is found in the test of bootstrap SCM in SCM HA. For details, please check the jira.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1369
## How was this patch tested?
CI
--
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] [ratis] bshashikant commented on pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #473:
URL: https://github.com/apache/ratis/pull/473#issuecomment-826588152
@szetszwo , can you please check this one?
--
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] [ratis] szetszwo merged pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #473:
URL: https://github.com/apache/ratis/pull/473
--
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] [ratis] GlenGeng commented on pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #473:
URL: https://github.com/apache/ratis/pull/473#issuecomment-826492764
@bshashikant @bharatviswa504 @szetszwo Can you please take a look ?
--
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] [ratis] szetszwo commented on a change in pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #473:
URL: https://github.com/apache/ratis/pull/473#discussion_r620114615
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
##########
@@ -431,7 +431,7 @@ private SnapshotInfo getLatestSnapshot() {
long getLatestInstalledSnapshotIndex() {
final TermIndex ti = latestInstalledSnapshot.get();
- return ti != null? ti.getIndex(): 0L;
+ return ti != null? ti.getIndex(): -1L;
Review comment:
Let's use `RaftLog.INVALID_LOG_INDEX` instead of -1.
```suggestion
return ti != null? ti.getIndex(): RaftLog.INVALID_LOG_INDEX;
```
##########
File path: ratis-server/src/main/java/org/apache/ratis/server/impl/ServerState.java
##########
@@ -440,7 +440,7 @@ long getLatestInstalledSnapshotIndex() {
*/
long getSnapshotIndex() {
final SnapshotInfo s = getLatestSnapshot();
- final long latestSnapshotIndex = s != null ? s.getIndex() : 0;
+ final long latestSnapshotIndex = s != null ? s.getIndex() : -1;
Review comment:
Let's use `RaftLog.INVALID_LOG_INDEX` instead of -1.
```suggestion
final long latestSnapshotIndex = s != null ? s.getIndex() : RaftLog.INVALID_LOG_INDEX;
```
--
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] [ratis] GlenGeng commented on pull request #473: RATIS-1369. When there is no snapshot, the return snapshotIndex should be -1, not 0.
Posted by GitBox <gi...@apache.org>.
GlenGeng commented on pull request #473:
URL: https://github.com/apache/ratis/pull/473#issuecomment-826492764
@bshashikant @bharatviswa504 @szetszwo Can you please take a look ?
--
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