ivandika3 commented on code in PR #10959:
URL: https://github.com/apache/ozone/pull/10959#discussion_r3754968967
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java:
##########
@@ -288,6 +288,10 @@ public void testInstallSnapshotWithClientWrite() throws
Exception {
});
List<String> newKeys = writeFuture.get();
+ // A write is acked once queued in the double buffer, but the applied index
+ // advances only after the buffer commits. Flush so it covers all newKeys.
+ leaderOM.awaitDoubleBufferFlush();
Review Comment:
Ok after revisiting the logic for applied index, Ratis has two applied index
concepts
- `BaseStateMachine`'s lastAppliedTermIndex : This is updated by the
`BaseStateMachine` implementation, for `OzoneManagerStateMachine`, it is
updated after RocksDB flush and reflected by the `transactionInfoTable`
- `StateMachineUpdater.appliedIndex`: This is the internal Ratis applied
index, this is updated internally by Ratis and not exposed by the user. Meaning
user will get the write reply after the StateMachineUpdater updates the
appliedIndex, but it doesn't mean that it has been persisted to
transactionInfoTable.
So I think the initial `awaitDoubleBufferFlush` might be ok since snapshot
is closely related to `BaseStateMachine`'s lastAppliedTermIndex. I don't like
current way of using Ratis commit index since we commitIndex is an internal
Raft concept and should not be exposed to user. User only cares about applied
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
