rpuch commented on code in PR #1280:
URL: https://github.com/apache/ignite-3/pull/1280#discussion_r1010539667
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionSnapshotStorageFactory.java:
##########
@@ -77,44 +73,34 @@ public PartitionSnapshotStorageFactory(
TopologyService topologyService,
OutgoingSnapshotsManager outgoingSnapshotsManager,
PartitionAccess partition,
- List<String> peers,
- List<String> learners,
Executor incomingSnapshotsExecutor
) {
this.topologyService = topologyService;
this.outgoingSnapshotsManager = outgoingSnapshotsManager;
this.partition = partition;
- this.peers = peers;
- this.learners = learners;
this.incomingSnapshotsExecutor = incomingSnapshotsExecutor;
// We must choose the minimum applied index for local recovery so that
we don't skip the raft commands for the storage with the
// lowest applied index and thus no data loss occurs.
- persistedRaftIndex = Math.min(
- partition.mvPartitionStorage().persistedIndex(),
- partition.txStatePartitionStorage().persistedIndex()
+ lastIncludedRaftIndex = Math.min(
+ partition.mvPartitionStorage().lastAppliedIndex(),
+ partition.txStatePartitionStorage().lastAppliedIndex()
);
}
@Override
- public PartitionSnapshotStorage createSnapshotStorage(String uri,
RaftOptions raftOptions) {
- SnapshotMeta snapshotMeta = new RaftMessagesFactory().snapshotMeta()
- .lastIncludedIndex(persistedRaftIndex)
- // According to the code of
org.apache.ignite.raft.jraft.core.NodeImpl.bootstrap, it's "dangerous" to init
term with a value
- // greater than 1. 0 value of persisted index means that the
underlying storage is empty.
- .lastIncludedTerm(persistedRaftIndex > 0 ? 1 : 0)
Review Comment:
Well, I just restored what JRaft seems to be doing by default with its
'local' snapshots.
1. When a snapshot is created, its meta is constructed with the real
index+term (term is not changed to 0/1). This meta is written to the snapshot
file
2. If, at node startup, the state is read from a snapshot, then the file is
opened and `SnapshotReader.load()` returns this meta (having real term,
probably a lot higher than 1)
This is what `StartupPartitionSnapshotReader.load()` will return: a meta
constructed from real index+term at the moment of snapshot start.
There is a comment in `NodeImpl` that says about danger, but the code at
that place is about bootstrapping an `FSMCaller` and not about opening a
snapshot.
And I don't understand what kind of an inconsistency (see the comment by
link) might arise if a node restored itself from a snapshot created with a
higher term: the 'leader' with a lower term cannot be a leader, that seems
correct. Can such a situation even happen that there was a leader with a higher
term (that created a snapshot with its term), and later there is a leader with
a lower term?
--
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]
