OneSizeFitsQuorum commented on PR #1207:
URL: https://github.com/apache/ratis/pull/1207#issuecomment-2576608710
@szetszwo
I don't think prematurely exiting the waitForCommit function has any impact,
because during continuous writes, applyIndex will always be smaller than
commitIndex, so the stateMachineUpdater will never get stuck in waitForCommit.
This ensures correctness for two reasons: first, before we execute the
checkAndTakeSnapshot function, we always run applyLog to ensure that applyIndex
reaches commitIndex; second, the semantics of takeSnapshot itself is based on
taking snapshots at applyIndex.
```Java
public void run() {
for(; state != State.STOP; ) {
try {
waitForCommit();
if (state == State.RELOAD) {
reload();
}
final MemoizedSupplier<List<CompletableFuture<Message>>> futures =
applyLog();
checkAndTakeSnapshot(futures);
if (shouldStop()) {
checkAndTakeSnapshot(futures);
stop();
}
} catch (Throwable t) {
if (t instanceof InterruptedException && state == State.STOP) {
Thread.currentThread().interrupt();
LOG.info("{} was interrupted. Exiting ...", this);
} else {
state = State.EXCEPTION;
LOG.error(this + " caught a Throwable.", t);
server.close();
}
}
}
}
```
```Java
* @return the largest index of the log entry that has been applied to the
* state machine and also included in the snapshot. Note the log
purge
* should be handled separately.
*/
// TODO: refactor this
long takeSnapshot() throws IOException;
```
Of course, your patch can also work perfectly fine, and I've already
modified it to match your approach.
--
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: issues-unsubscr...@ratis.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
