lokeshj1703 commented on a change in pull request #3049:
URL: https://github.com/apache/ozone/pull/3049#discussion_r808932234
##########
File path:
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -214,15 +215,12 @@ public void
unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced() {
balancerConfiguration.setThreshold(99.99);
containerBalancer.start(balancerConfiguration);
- // waiting for balance completed.
- // TODO: this is a temporary implementation for now
- // modify this after balancer is fully completed
- try {
- Thread.sleep(100);
- } catch (InterruptedException e) { }
+ sleepWhileBalancing(100);
containerBalancer.stop();
+ ContainerBalancerMetrics metrics = containerBalancer.getMetrics();
Assert.assertEquals(0, containerBalancer.getUnBalancedNodes().size());
+ Assert.assertEquals(0, metrics.getDatanodesNumUnbalanced());
Review comment:
Can we also assert that unbalanced datanodes and size is some expected
number in a simulated test, if it is not very difficult to add?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java
##########
@@ -36,17 +36,27 @@
private final MetricsSystem ms;
- @Metric(about = "The amount of Gigabytes that Container Balancer moved" +
- " in the last iteration.")
- private MutableCounterLong dataSizeMovedGB;
+ @Metric(about = "Amount of Gigabytes that Container Balancer moved" +
Review comment:
Let's have consistent naming for these metrics? `countIterations` to
`numIterations`.
Like using num prefix. `dataSizeCausingUnbalanceGB` to
`dataSizeUnbalancedGB` etc.
##########
File path:
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -305,16 +302,14 @@ public void
containerBalancerShouldObeyMaxSizeToMoveLimit() {
balancerConfiguration.setIterations(1);
containerBalancer.start(balancerConfiguration);
- // waiting for balance completed.
- // TODO: this is a temporary implementation for now
- // modify this after balancer is fully completed
- try {
- Thread.sleep(1000);
- } catch (InterruptedException e) { }
+ sleepWhileBalancing(500);
// balancer should not have moved more size than the limit
Assert.assertFalse(containerBalancer.getSizeMovedPerIteration() >
10 * OzoneConsts.GB);
+ Assert.assertFalse(
+ containerBalancer.getMetrics().getDataSizeMovedGBInLatestIteration() >
+ 10);
Review comment:
Could we also add assertion that metric is greater than 0?
--
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]
