You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Montura (via GitHub)" <gi...@apache.org> on 2024/09/04 20:21:09 UTC

[PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   In [PR for HDDS-9889](https://github.com/apache/ozone/pull/5758) we [discussed](https://github.com/apache/ozone/pull/5758#pullrequestreview-1963251436) with [Siddhant Sangwan](https://issues.apache.org/jira/secure/ViewProfile.jspa?name=siddhant) that tests form `org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask` could be refactored using `MockedSCM` class (introduced in [HDDS-9889](https://issues.apache.org/jira/browse/HDDS-9889))
   
   Some work has been already done in:
   1. https://github.com/apache/ozone/pull/5758
   2. https://github.com/apache/ozone/pull/6537
   3. https://github.com/apache/ozone/pull/6734
   
   ## What changes were proposed in this pull request?
   
   1. Refactor some tests from `org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask` 
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-11410
   
   ## How was this patch tested?
   
   Use standalone tests
   


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   > @Montura `TestContainerBalancerDatanodeNodeLimit#checkIterationResultException`, when we run the test class, not just a method.
   > 
   > https://github.com/adoroszlai/ozone/actions/runs/12152868992/job/33890752644#step:3:21
   
   I'll take a look today 


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Sure, I'll restart


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Thanks @ivandika3 for the info.  Launched a [run](https://github.com/adoroszlai/ozone/actions/runs/12152868992) for the entire test class.
   
   [Previous run](https://github.com/unisonteam/ozone/actions/runs/12056169848) only covered a single method, there may be interaction between test cases.


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Thanks @Montura.  Created HDDS-11855 for the failure.


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @Montura, thank you.  Much better:
   
   ```
   [INFO] Tests run: 16, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.162 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   ```
   
   https://github.com/unisonteam/ozone/actions/runs/12056169848/job/33618737808#step:7:1654


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @Montura no, I only meant that based on [this conversation](https://github.com/apache/ozone/pull/7156#discussion_r1744938637) something needs to be done about setting threshold.  If you have addressed that, please reply `/ready` to remove the `pending` label.


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @adoroszlai , you mean explicitly write ```config.setThreshold(...)``` in each test case?


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   > Marking this issue as un-mergeable as requested.
   > 
   > Please use `/ready` comment when it's resolved.
   > 
   > Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)
   > 
   > > set threshold in tests
   
   /ready


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java:
##########
@@ -208,15 +186,7 @@ public void balancerShouldObeyMaxSizeLeavingSourceLimit(@Nonnull MockedSCM mocke
   @ParameterizedTest(name = "MockedSCM #{index}: {0}")
   @MethodSource("createMockedSCMs")
   public void initializeIterationShouldUpdateUnBalancedNodesWhenThresholdChanges(@Nonnull MockedSCM mockedSCM) {
-    ContainerBalancerConfiguration config = new OzoneConfiguration().getObject(ContainerBalancerConfiguration.class);
-    int nodeCount = mockedSCM.getCluster().getNodeCount();
-    if (nodeCount < DATANODE_COUNT_LIMIT_FOR_SMALL_CLUSTER) {
-      config.setMaxDatanodesPercentageToInvolvePerIteration(100);
-    }
-    config.setThreshold(10);

Review Comment:
   /ready



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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java:
##########
@@ -208,15 +186,7 @@ public void balancerShouldObeyMaxSizeLeavingSourceLimit(@Nonnull MockedSCM mocke
   @ParameterizedTest(name = "MockedSCM #{index}: {0}")
   @MethodSource("createMockedSCMs")
   public void initializeIterationShouldUpdateUnBalancedNodesWhenThresholdChanges(@Nonnull MockedSCM mockedSCM) {
-    ContainerBalancerConfiguration config = new OzoneConfiguration().getObject(ContainerBalancerConfiguration.class);
-    int nodeCount = mockedSCM.getCluster().getNodeCount();
-    if (nodeCount < DATANODE_COUNT_LIMIT_FOR_SMALL_CLUSTER) {
-      config.setMaxDatanodesPercentageToInvolvePerIteration(100);
-    }
-    config.setThreshold(10);

Review Comment:
   @Tejaskriya , thank for a review. I forgot about **threshold** thanks, I'll fix 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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   > https://github.com/unisonteam/ozone/actions/runs/12050928627
   > TestContainerBalancerDatanodeNodeLimit#checkIterationResultException[master]-10x10
   
   Thanks.  `Git Ref (Branch/Commit_ID/Tag)` should be set to `container-balancer-tests`, but here it was testing `master`, where `TestContainerBalancerDatanodeNodeLimit` does not exist.
   
   Normally the workflow would report "no tests were run" error in such case.  However, `master` in your fork (11850f5f9ba6d21f1665b2c07a1ebb73a8623cfa) is older than the commit where this sanity check was introduced (4195ef57a3a29aab3a79747147d0d73dc63c34af).
   
   Can you please launch a new run, with `Git Ref (Branch/Commit_ID/Tag): container-balancer-tests`?


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   UPD:
   - `initializeIterationShouldUpdateUnBalancedNodesWhenThresholdChanges` : reduce iterations 50 -> 10
   - `checkIterationResultTimeout` : increase `maxEnteringSize`, decrease `timeouts`


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @ivandika3 , which test (or tests) is (are) flaky now?
   
   I can look/run locally and prepare hotfix or tune the limits (as I've done before)


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Seems TestContainerBalancerDatanodeNodeLimit is flaky 
   
   https://github.com/apache/ozone/actions/runs/12151251117/job/33885814517


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @Tejaskriya, take a look please


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   I'll take a look at failed `checkIterationResultException` test


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Ready to merge?


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @Montura `TestContainerBalancerDatanodeNodeLimit#checkIterationResultException`, when we run the test class, not just a method.
   
   https://github.com/adoroszlai/ozone/actions/runs/12152868992/job/33890752644#step:3:21


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @adoroszlai , I'm still investigating. For now the test still is flacky 
   https://github.com/unisonteam/ozone/actions/runs/12250923249/job/34175235709


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Thanks @Montura for updating the tests.  You can check for flakiness by running `TestContainerBalancerDatanodeNodeLimit` with [flaky-test-check](https://github.com/unisonteam/ozone/actions/workflows/intermittent-test-check.yml) (a run with 10 splits and 10 iterations would be nice).


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Thanks @Montura for the patch.
   
   > @siddhantsangwan @sumitagrawl would you like to take a look?
   
   I guess not.


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   > There are 3 possible solutions:
   > 
   > 1. to reduce the total amount of arguments for `checkIterationResultTimeout` and `checkIterationResultException` tests.
   > 2. to decrease the `timeout` time limit
   > 3. Do items 1 and 2 simultaneously
   
   Can you please explore how much `timeout` can be decreased without introducing flakiness?  Depending on that, we'd need to go with one of the solutions.


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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


##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java:
##########
@@ -208,15 +186,7 @@ public void balancerShouldObeyMaxSizeLeavingSourceLimit(@Nonnull MockedSCM mocke
   @ParameterizedTest(name = "MockedSCM #{index}: {0}")
   @MethodSource("createMockedSCMs")
   public void initializeIterationShouldUpdateUnBalancedNodesWhenThresholdChanges(@Nonnull MockedSCM mockedSCM) {
-    ContainerBalancerConfiguration config = new OzoneConfiguration().getObject(ContainerBalancerConfiguration.class);
-    int nodeCount = mockedSCM.getCluster().getNodeCount();
-    if (nodeCount < DATANODE_COUNT_LIMIT_FOR_SMALL_CLUSTER) {
-      config.setMaxDatanodesPercentageToInvolvePerIteration(100);
-    }
-    config.setThreshold(10);

Review Comment:
   Setting the threshold has been removed here and in few more tests below, but in the newly introduced ContainerBalancerConfigBuilder, we aren't setting it. Is this intentional?



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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   /pending set threshold in tests


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Looks like this increases test run time from:
   
   ```
   [INFO] Tests run: 240, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 34.125 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.686 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
   ```
   
   to:
   
   ```
   [INFO] Tests run: 304, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 70.262 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.735 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
   ```
   
   So 2x for `TestContainerBalancerDatanodeNodeLimit` and x/2 for `TestContainerBalancerTask`, but the problem is that the increase is an order of magnitude more.
   
   The following test cases take more than 1 second:
   
   ```
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[1]  Time elapsed: 1.028 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[2]  Time elapsed: 1.024 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[3]  Time elapsed: 1.024 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[4]  Time elapsed: 1.025 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[5]  Time elapsed: 1.025 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[6]  Time elapsed: 1.025 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[1]  Time elapsed: 1.026 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[2]  Time elapsed: 1.024 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[3]  Time elapsed: 1.029 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[4]  Time elapsed: 1.026 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[5]  Time elapsed: 1.029 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[6]  Time elapsed: 1.03 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[7]  Time elapsed: 1.035 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[8]  Time elapsed: 1.05 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[9]  Time elapsed: 1.039 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[10]  Time elapsed: 1.078 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[11]  Time elapsed: 1.046 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[12]  Time elapsed: 1.053 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[13]  Time elapsed: 1.062 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[14]  Time elapsed: 1.074 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[15]  Time elapsed: 1.094 s
   org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[16]  Time elapsed: 1.157 s
   ```


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   > Looks like this increases test run time from:
   > 
   > ```
   > [INFO] Tests run: 240, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 34.125 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   > [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.686 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
   > ```
   > 
   > to:
   > 
   > ```
   > [INFO] Tests run: 304, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 70.262 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   > [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.735 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
   > ```
   > 
   > So 2x for `TestContainerBalancerDatanodeNodeLimit` and x/2 for `TestContainerBalancerTask`, but the problem is that the increase is an order of magnitude more.
   > 
   > The following test cases take more than 1 second:
   > 
   > ```
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[1]  Time elapsed: 1.028 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[2]  Time elapsed: 1.024 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[3]  Time elapsed: 1.024 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[4]  Time elapsed: 1.025 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[5]  Time elapsed: 1.025 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultException(MockedSCM)[6]  Time elapsed: 1.025 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[1]  Time elapsed: 1.026 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[2]  Time elapsed: 1.024 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[3]  Time elapsed: 1.029 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[4]  Time elapsed: 1.026 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[5]  Time elapsed: 1.029 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[6]  Time elapsed: 1.03 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[7]  Time elapsed: 1.035 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[8]  Time elapsed: 1.05 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[9]  Time elapsed: 1.039 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[10]  Time elapsed: 1.078 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[11]  Time elapsed: 1.046 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[12]  Time elapsed: 1.053 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[13]  Time elapsed: 1.062 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[14]  Time elapsed: 1.074 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[15]  Time elapsed: 1.094 s
   > org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit.checkIterationResultTimeout(MockedSCM)[16]  Time elapsed: 1.157 s
   > ```
   
   There are 3 possible solutions:
    1. to reduce the total amount of arguments for `checkIterationResultTimeout` and `checkIterationResultException` tests. 
    2. to decrease the `timeout` time limit
    3. Do items 1 and 2 simultaneously 
   


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @adoroszlai , passed https://github.com/unisonteam/ozone/actions/runs/12050928627


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   Thanks @Montura for reducing test time:
   
   ```
   Tests run: 304, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.946 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerDatanodeNodeLimit
   Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.542 s - in org.apache.hadoop.hdds.scm.container.balancer.TestContainerBalancerTask
   ```


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


Re: [PR] HDDS-11410. Refactoring more tests from TestContainerBalancerTask [ozone]

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

   @siddhantsangwan @sumitagrawl would you like to 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