Re: [PR] HDDS-6115. Intermittent failure in TestContainerStateMachineFailures#testApplyTransactionIdempotencyWithClosedContainer [ozone]

["adoroszlai (via GitHub)" <gi...@apache.org>]

adoroszlai commented on code in PR #5482:
URL: https://github.com/apache/ozone/pull/5482#discussion_r1371395902


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachineFailures.java:
##########
@@ -549,6 +552,22 @@ public void testApplyTransactionIdempotencyWithClosedContainer()
     } finally {
       xceiverClientManager.releaseClient(xceiverClient, false);
     }
+    // This is just an attempt to wait for an asynchronous call from Ratis API
+    // to updateIncreasingly to finish as part of flaky test issue "HDDS-6115"
+    // This doesn't solve the problem completely but reduce the failure ratio.
+    try {
+      GenericTestUtils.waitFor((() -> {
+        try {
+          return markIndex1 != StatemachineImplTestUtil
+                  .findLatestSnapshot(storage).getIndex();
+        } catch (IOException e) {
+          // No action needed. The test case is going to fail at assertion.
+          return true;
+        }
+      }), 1000, 30000);
+    } catch (Exception e) {
+      // No action needed. The test case is going to fail at assertion.
+    }

Review Comment:
   `waitFor()` prints a dump of all threads if it times out, which can be helpful in debugging.  So we shouldn't wrap `waitFor()` in `try-catch`, rather let the test fail directly due to the `TimeoutException` it throws.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachineFailures.java:
##########
@@ -549,6 +552,22 @@ public void testApplyTransactionIdempotencyWithClosedContainer()
     } finally {
       xceiverClientManager.releaseClient(xceiverClient, false);
     }
+    // This is just an attempt to wait for an asynchronous call from Ratis API
+    // to updateIncreasingly to finish as part of flaky test issue "HDDS-6115"
+    // This doesn't solve the problem completely but reduce the failure ratio.
+    try {
+      GenericTestUtils.waitFor((() -> {
+        try {
+          return markIndex1 != StatemachineImplTestUtil
+                  .findLatestSnapshot(storage).getIndex();

Review Comment:
   ```suggestion
                 .findLatestSnapshot(storage).getIndex();
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachineFailures.java:
##########
@@ -518,6 +519,8 @@ public void testApplyTransactionIdempotencyWithClosedContainer()
     stateMachine.takeSnapshot();
     Assert.assertTrue(parentPath.getParent().toFile().listFiles().length > 0);
     Assert.assertNotNull(snapshot);
+    long markIndex1 = StatemachineImplTestUtil.findLatestSnapshot(storage)
+            .getIndex();

Review Comment:
   Nit: continuation line should be indented by 4 spaces instead of 8, please adjust your IDE settings.
   
   ```suggestion
           .getIndex();
   ```



-- 
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-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org