ivandika3 commented on code in PR #10959:
URL: https://github.com/apache/ozone/pull/10959#discussion_r3733758774


##########
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:
   Is this the same workaround of https://github.com/apache/ozone/pull/10748 
(i.e. flush the double buffer so that the key is visible)? If that's so, I 
don't agree with this as mentioned in that ticket since this means the OM 
linearizability can be violated. We should not need to wait for double buffer 
flush for the key to be visible. We should fix the root cause, not just trying 
to pass the tests. Feel free to correct me if I'm mistaken about the OM 
consistency guarantee since it's never properly expounded or proven by the 
original author (https://ozone.apache.org/docs/1.4.1/design/omha.html). 
   
   The only time manually awaitDoubleBufferFlush is valid is for OMResponse 
unit test, not in integration tests. I saw a lot of `awaitDoubleBufferFlush`(or 
worse, sleep workaround) in integration test which is worrying.
   
   But if it's because of the appliedIndex check
   
   ```java
   // The recently started OM should be lagging behind the leader OM.
       // Wait & for follower to update transactions to leader snapshot index.
       // Timeout error if follower does not load update within 3s
       GenericTestUtils.waitFor(() -> {
         return 
followerOM.getOmRatisServer().getLastAppliedTermIndex().getIndex()
             >= leaderOMSnapshotIndex - 1;
       }, 100, 30_000);
   ```
   
   I might be fine with it. In that case, please move the 
`awaitDoubleBufferFlush` after the key visibility check.



-- 
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]

Reply via email to