You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ArafatKhan2198 (via GitHub)" <gi...@apache.org> on 2023/04/04 14:35:46 UTC

[GitHub] [ozone] ArafatKhan2198 opened a new pull request, #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

ArafatKhan2198 opened a new pull request, #4534:
URL: https://github.com/apache/ozone/pull/4534

   ## What changes were proposed in this pull request?
   
   The testDeletedContainerCount() method of TestContainerStateCounts is considered flaky due to the processing order of the container report. If the pipeline does not exist, then the container is not added until the next container report is processed.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-8374
   
   ## How was this patch tested?
   
   Ran the UT several times to make sure it's not flaky 


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1496152999

   @ArafatKhan2198 seems like the test failed in the first run:
   
   ```
   Error:  Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 37.517 s <<< FAILURE! - in org.apache.hadoop.ozone.recon.api.TestContainerStateCounts
   Error:  org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.testContainerStatsCount  Time elapsed: 37.468 s  <<< ERROR!
   java.util.concurrent.TimeoutException: timeout: after 30000 millis
   	at org.apache.ozone.test.LambdaTestUtils$GenerateTimeout.evaluate(LambdaTestUtils.java:677)
   	at org.apache.ozone.test.LambdaTestUtils.await(LambdaTestUtils.java:154)
   	at org.apache.ozone.test.LambdaTestUtils.await(LambdaTestUtils.java:192)
   	at org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.waitAndCheckConditionAfterHeartbeat(TestContainerStateCounts.java:413)
   	at org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.testContainerStatsCount(TestContainerStateCounts.java:369)
   ```
   
   https://github.com/ArafatKhan2198/ozone/actions/runs/4608724963/jobs/8144896789#step:6:2706


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1502827780

   Thanks @ArafatKhan2198 for updating the patch.  Have you run repeated tests in CI?  Can you please share the link to the CI run?


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1504848625

   Please try to avoid force-push when updating the PR.  Here are some great articles that explain why:
   
   https://developers.mattermost.com/blog/submitting-great-prs/#4-avoid-force-pushing
   https://www.freecodecamp.org/news/optimize-pull-requests-for-reviewer-happiness#request-a-review


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

[GitHub] [ozone] ArafatKhan2198 commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1505348730

   > @ArafatKhan2198 I may be missing something, but the latest commit [3fb721a](https://github.com/apache/ozone/commit/3fb721a8abb801bc868901c05c4105e349869467) doesn't seem to be related to the fix.
   
   Hi @adoroszlai 
   In the pull request [HDDS-7994](https://github.com/apache/ozone/pull/4406), I have added a few more parameters to the JSON response of the clusterStateEndpoint of Recon. One of these parameters is keys pending for deletion. When we decided to expose this information to the UI, there was a [suggestion](https://github.com/apache/ozone/pull/4519#discussion_r1156939996) to change the response parameter "**deletedKeys**" to "**keysAwaitingDeletion**". This parameter shows the keys in the deleted table waiting to be deleted by the keyDeletionService running in the background. Since HDDS-7994 has already been merged, I did not get a chance to change the response. Therefore, I have decided to incorporate this small name change in this PR.


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

[GitHub] [ozone] adoroszlai commented on a diff in pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4534:
URL: https://github.com/apache/ozone/pull/4534#discussion_r1157387009


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerStateCounts.java:
##########
@@ -387,4 +397,20 @@ public void testDeletedContainerCount() throws Exception {
     Assertions.assertEquals(3, clusterStateResponse1.getOpenContainers());
   }
 
+  private void waitAndCheckConditionAfterHeartbeat(Callable<Boolean> check)
+      throws Exception {
+    // if container report is processed first, and pipeline does not exist
+    // then container is not added until the next container report is processed
+    StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto
+        heartbeatRequestProto =
+        StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto.newBuilder()

Review Comment:
   ```
   hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerStateCounts.java
    406: Line is longer than 80 characters (found 84).
   ```



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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1505363285

   > Therefore, I have decided to incorporate this small name change in this PR.
   
   It would be better to avoid incorporating unrelated changes.
   
   1. We already had successful CI for this PR, could have been merged.  Now we need to trigger another run.
   2. Commit message would be confusing (fix flaky test vs. rename JSON element).
   3. CI needs to run fewer checks if only unit test is changed.


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

[GitHub] [ozone] ArafatKhan2198 commented on a diff in pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4534:
URL: https://github.com/apache/ozone/pull/4534#discussion_r1157392736


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerStateCounts.java:
##########
@@ -387,4 +397,20 @@ public void testDeletedContainerCount() throws Exception {
     Assertions.assertEquals(3, clusterStateResponse1.getOpenContainers());
   }
 
+  private void waitAndCheckConditionAfterHeartbeat(Callable<Boolean> check)
+      throws Exception {
+    // if container report is processed first, and pipeline does not exist
+    // then container is not added until the next container report is processed
+    StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto
+        heartbeatRequestProto =
+        StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto.newBuilder()

Review Comment:
   Thanks for pointing it out. For some reason the `./checkStyle` script in dev tools did not find it out. But I have changed it 
   



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

[GitHub] [ozone] ArafatKhan2198 commented on a diff in pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on code in PR #4534:
URL: https://github.com/apache/ozone/pull/4534#discussion_r1157392736


##########
hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerStateCounts.java:
##########
@@ -387,4 +397,20 @@ public void testDeletedContainerCount() throws Exception {
     Assertions.assertEquals(3, clusterStateResponse1.getOpenContainers());
   }
 
+  private void waitAndCheckConditionAfterHeartbeat(Callable<Boolean> check)
+      throws Exception {
+    // if container report is processed first, and pipeline does not exist
+    // then container is not added until the next container report is processed
+    StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto
+        heartbeatRequestProto =
+        StorageContainerDatanodeProtocolProtos.SCMHeartbeatRequestProto.newBuilder()

Review Comment:
   Thanks for pointing it out. I have fixed it 



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

[GitHub] [ozone] adoroszlai merged pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4534:
URL: https://github.com/apache/ozone/pull/4534


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

[GitHub] [ozone] ArafatKhan2198 commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1503653240

   > https://github.com/adoroszlai/hadoop-ozone/actions/runs/4610115793
   
   Here you go @adoroszlai  :- https://github.com/ArafatKhan2198/ozone/actions/runs/4669283289
   


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

[GitHub] [ozone] ArafatKhan2198 commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1503660543

   @devmadhuu @adoroszlai Can you please take a look 


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1505332194

   @ArafatKhan2198 I may be missing something, but the latest commit 3fb721a8abb801bc868901c05c4105e349869467 doesn't seem to be related to the fix.


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1496322131

   Thanks @ArafatKhan2198 for the update, but the test is still flaky, failed in 14/100 runs:
   
   https://github.com/adoroszlai/hadoop-ozone/actions/runs/4610115793


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

[GitHub] [ozone] ArafatKhan2198 commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1496274932

   > @ArafatKhan2198 seems like the test failed in the first run:
   > 
   > ```
   > Error:  Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 37.517 s <<< FAILURE! - in org.apache.hadoop.ozone.recon.api.TestContainerStateCounts
   > Error:  org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.testContainerStatsCount  Time elapsed: 37.468 s  <<< ERROR!
   > java.util.concurrent.TimeoutException: timeout: after 30000 millis
   > 	at org.apache.ozone.test.LambdaTestUtils$GenerateTimeout.evaluate(LambdaTestUtils.java:677)
   > 	at org.apache.ozone.test.LambdaTestUtils.await(LambdaTestUtils.java:154)
   > 	at org.apache.ozone.test.LambdaTestUtils.await(LambdaTestUtils.java:192)
   > 	at org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.waitAndCheckConditionAfterHeartbeat(TestContainerStateCounts.java:413)
   > 	at org.apache.hadoop.ozone.recon.api.TestContainerStateCounts.testContainerStatsCount(TestContainerStateCounts.java:369)
   > ```
   > 
   > https://github.com/ArafatKhan2198/ozone/actions/runs/4608724963/jobs/8144896789#step:6:2706
   
   My apologise @adoroszlai. By mistake, I reverted back to the previous value after running the tests successfully. Please run the tests now. I have already run them a total of 20 times, and they have passed.
   
   ![image](https://user-images.githubusercontent.com/98023601/229857710-c2a73209-2f9f-48c7-8e6a-4ff5a9cf1f65.png)
   
   
   
   


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

[GitHub] [ozone] ArafatKhan2198 commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "ArafatKhan2198 (via GitHub)" <gi...@apache.org>.
ArafatKhan2198 commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1505384456

   > > Therefore, I have decided to incorporate this small name change in this PR.
   > 
   > It would be better to avoid incorporating unrelated changes.
   > 
   > 1. We already had successful CI for this PR, could have been merged.  Now we need to trigger another run.
   > 2. Commit message would be confusing (fix flaky test vs. rename JSON element).
   > 3. CI needs to run fewer checks if only unit test is changed.
   
   Understood, I have removed those changes. 
   Thanks for the feedback


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

[GitHub] [ozone] adoroszlai commented on pull request #4534: HDDS-8374. Fix Flaky Unit Test for TestContainerStateCounts in Recon

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4534:
URL: https://github.com/apache/ozone/pull/4534#issuecomment-1503657840

   Thanks @ArafatKhan2198 for updating the patch and for repeated runs, the results look good.  I'll defer review of the actual fix to folks more familiar with Recon.


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