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. Please correct me if I'm wrong 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 test triggered flush 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 this 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