szetszwo commented on code in PR #643:
URL: https://github.com/apache/ratis/pull/643#discussion_r869072974
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java:
##########
@@ -49,15 +49,18 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
+import static org.apache.ratis.server.raftlog.RaftLog.INVALID_LOG_INDEX;
+
class SnapshotInstallationHandler {
static final Logger LOG =
LoggerFactory.getLogger(SnapshotInstallationHandler.class);
private final RaftServerImpl server;
private final ServerState state;
private final boolean installSnapshotEnabled;
- private final AtomicLong inProgressInstallSnapshotIndex = new AtomicLong();
- private final AtomicReference<TermIndex> installedSnapshotTermIndex = new
AtomicReference<>(TermIndex.valueOf(0,0));
+ private final AtomicLong inProgressInstallSnapshotIndex = new
AtomicLong(INVALID_LOG_INDEX);
+ private final AtomicReference<TermIndex> installedSnapshotTermIndex =
+ new AtomicReference<>(TermIndex.valueOf(0, INVALID_LOG_INDEX));
Review Comment:
Let's add an INVALID_TERM_INDEX constant.
```
static final TermIndex INVALID_TERM_INDEX = TermIndex.valueOf(0,
INVALID_LOG_INDEX);
```
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java:
##########
@@ -206,16 +209,16 @@ private InstallSnapshotReplyProto
notifyStateMachineToInstallSnapshot(
state.setLeader(leaderId, "installSnapshot");
server.updateLastRpcTime(FollowerState.UpdateType.INSTALL_SNAPSHOT_NOTIFICATION);
- if (inProgressInstallSnapshotIndex.compareAndSet(0,
firstAvailableLogIndex)) {
+ if (inProgressInstallSnapshotIndex.compareAndSet(INVALID_LOG_INDEX,
firstAvailableLogIndex)) {
LOG.info("{}: Received notification to install snapshot at index {}",
getMemberId(), firstAvailableLogIndex);
// Check if snapshot index is already at par or ahead of the first
// available log index of the Leader.
final long snapshotIndex = state.getLog().getSnapshotIndex();
- if (snapshotIndex + 1 >= firstAvailableLogIndex &&
firstAvailableLogIndex > 0) {
+ if (snapshotIndex >= firstAvailableLogIndex && firstAvailableLogIndex
> INVALID_LOG_INDEX) {
Review Comment:
snapshotIndex + 1 seems correct since snapshotIndex is the last index in a
snapshot. Then, leader can send log starting at firstAvailableLogIndex to this
server.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/SnapshotInstallationHandler.java:
##########
@@ -284,31 +287,31 @@ private InstallSnapshotReplyProto
notifyStateMachineToInstallSnapshot(
}
final long inProgressInstallSnapshotIndexValue =
getInProgressInstallSnapshotIndex();
- Preconditions.assertTrue(
- inProgressInstallSnapshotIndexValue <= firstAvailableLogIndex &&
inProgressInstallSnapshotIndexValue > 0,
+ Preconditions.assertTrue(inProgressInstallSnapshotIndexValue <=
firstAvailableLogIndex
+ && inProgressInstallSnapshotIndexValue > INVALID_LOG_INDEX,
"inProgressInstallSnapshotRequest: %s is not eligible,
firstAvailableLogIndex: %s",
getInProgressInstallSnapshotIndex(), firstAvailableLogIndex);
// If the snapshot is null or unavailable, return SNAPSHOT_UNAVAILABLE.
if (isSnapshotNull.compareAndSet(true, false)) {
LOG.info("{}: InstallSnapshot notification result: {}", getMemberId(),
InstallSnapshotResult.SNAPSHOT_UNAVAILABLE);
- inProgressInstallSnapshotIndex.set(0);
+ inProgressInstallSnapshotIndex.set(INVALID_LOG_INDEX);
return ServerProtoUtils.toInstallSnapshotReplyProto(leaderId,
getMemberId(),
currentTerm, InstallSnapshotResult.SNAPSHOT_UNAVAILABLE, -1);
}
// If a snapshot has been installed, return SNAPSHOT_INSTALLED with the
installed snapshot index and reset
- // installedSnapshotIndex to (0,0).
+ // installedSnapshotIndex to (0,-1).
final TermIndex latestInstalledSnapshotTermIndex =
this.installedSnapshotTermIndex
- .getAndSet(TermIndex.valueOf(0,0));
- if (latestInstalledSnapshotTermIndex.getIndex() > 0) {
+ .getAndSet(TermIndex.valueOf(0, INVALID_LOG_INDEX));
+ if (latestInstalledSnapshotTermIndex.getIndex() > INVALID_LOG_INDEX) {
Review Comment:
Let's use INVALID_TERM_INDEX.
```
// installedSnapshotTermIndex to INVALID_TERM_INDEX.
final TermIndex latestInstalledSnapshotTermIndex =
this.installedSnapshotTermIndex.getAndSet(INVALID_TERM_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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
