sumitagrawl commented on code in PR #3782:
URL: https://github.com/apache/ozone/pull/3782#discussion_r988740394
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java:
##########
@@ -212,559 +206,102 @@ public void setup() throws IOException,
NodeNotFoundException,
}
@Test
- public void testCalculationOfUtilization() {
- Assertions.assertEquals(nodesInCluster.size(), nodeUtilizations.size());
- for (int i = 0; i < nodesInCluster.size(); i++) {
- Assertions.assertEquals(nodeUtilizations.get(i),
- nodesInCluster.get(i).calculateUtilization(), 0.0001);
- }
-
+ public void testShouldRun() throws Exception {
+ boolean doRun = containerBalancer.shouldRun();
// should be equal to average utilization of the cluster
- Assertions.assertEquals(averageUtilization,
- containerBalancer.calculateAvgUtilization(nodesInCluster), 0.0001);
- }
-
- /**
- * Checks whether ContainerBalancer is correctly updating the list of
- * unBalanced nodes with varying values of Threshold.
- */
- @Test
- public void
- initializeIterationShouldUpdateUnBalancedNodesWhenThresholdChanges()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- List<DatanodeUsageInfo> expectedUnBalancedNodes;
- List<DatanodeUsageInfo> unBalancedNodesAccordingToBalancer;
-
- // check for random threshold values
- for (int i = 0; i < 50; i++) {
- double randomThreshold = RANDOM.nextDouble() * 100;
-
- balancerConfiguration.setThreshold(randomThreshold);
- startBalancer(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) { }
-
- expectedUnBalancedNodes =
- determineExpectedUnBalancedNodes(randomThreshold);
- unBalancedNodesAccordingToBalancer =
- containerBalancer.getUnBalancedNodes();
-
- stopBalancer();
- Assertions.assertEquals(
- expectedUnBalancedNodes.size(),
- unBalancedNodesAccordingToBalancer.size());
-
- for (int j = 0; j < expectedUnBalancedNodes.size(); j++) {
- Assertions.assertEquals(
- expectedUnBalancedNodes.get(j).getDatanodeDetails(),
- unBalancedNodesAccordingToBalancer.get(j).getDatanodeDetails());
- }
- }
+ Assertions.assertEquals(doRun, false);
+ containerBalancer.saveConfiguration(balancerConfiguration, true, 0);
+ doRun = containerBalancer.shouldRun();
+ Assertions.assertEquals(doRun, true);
+ containerBalancer.saveConfiguration(balancerConfiguration, false, 0);
+ doRun = containerBalancer.shouldRun();
+ Assertions.assertEquals(doRun, false);
}
- /**
- * Checks whether the list of unBalanced nodes is empty when the cluster is
- * balanced.
- */
@Test
- public void unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- balancerConfiguration.setThreshold(99.99);
- startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(100);
-
- stopBalancer();
- ContainerBalancerMetrics metrics = containerBalancer.getMetrics();
- Assertions.assertEquals(0, containerBalancer.getUnBalancedNodes().size());
- Assertions.assertEquals(0, metrics.getNumDatanodesUnbalanced());
- }
-
- /**
- * ContainerBalancer should not involve more datanodes than the
- * maxDatanodesRatioToInvolvePerIteration limit.
- */
- @Test
- public void containerBalancerShouldObeyMaxDatanodesToInvolveLimit()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- int percent = 20;
- balancerConfiguration.setMaxDatanodesPercentageToInvolvePerIteration(
- percent);
- balancerConfiguration.setMaxSizeToMovePerIteration(100 * STORAGE_UNIT);
- balancerConfiguration.setThreshold(1);
- balancerConfiguration.setIterations(1);
- startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(500);
-
- int number = percent * numberOfNodes / 100;
- ContainerBalancerMetrics metrics = containerBalancer.getMetrics();
- Assertions.assertFalse(
- containerBalancer.getCountDatanodesInvolvedPerIteration() > number);
- Assertions.assertTrue(
- metrics.getNumDatanodesInvolvedInLatestIteration() > 0);
- Assertions.assertFalse(
- metrics.getNumDatanodesInvolvedInLatestIteration() > number);
- stopBalancer();
- }
+ public void testStartBalancerStop() throws Exception {
+ Mockito.when(replicationManager.move(Mockito.any(ContainerID.class),
+ Mockito.any(DatanodeDetails.class),
+ Mockito.any(DatanodeDetails.class)))
+ .thenReturn(genCompletableFuture(1000));
- @Test
- public void containerBalancerShouldSelectOnlyClosedContainers()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- // make all containers open, balancer should not select any of them
- for (ContainerInfo containerInfo : cidToInfoMap.values()) {
- containerInfo.setState(HddsProtos.LifeCycleState.OPEN);
- }
balancerConfiguration.setThreshold(10);
- startBalancer(balancerConfiguration);
- sleepWhileBalancing(500);
- stopBalancer();
-
- // balancer should have identified unbalanced nodes
- Assertions.assertFalse(containerBalancer.getUnBalancedNodes().isEmpty());
- // no container should have been selected
- Assertions.assertTrue(containerBalancer.getContainerToSourceMap()
- .isEmpty());
- /*
- Iteration result should be CAN_NOT_BALANCE_ANY_MORE because no container
- move is generated
- */
- Assertions.assertEquals(
- ContainerBalancer.IterationResult.CAN_NOT_BALANCE_ANY_MORE,
- containerBalancer.getIterationResult());
-
- // now, close all containers
- for (ContainerInfo containerInfo : cidToInfoMap.values()) {
- containerInfo.setState(HddsProtos.LifeCycleState.CLOSED);
- }
- startBalancer(balancerConfiguration);
- sleepWhileBalancing(500);
- stopBalancer();
-
- // check whether all selected containers are closed
- for (ContainerID cid:
- containerBalancer.getContainerToSourceMap().keySet()) {
- Assertions.assertSame(
- cidToInfoMap.get(cid).getState(), HddsProtos.LifeCycleState.CLOSED);
- }
- }
-
- @Test
- public void containerBalancerShouldObeyMaxSizeToMoveLimit()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- balancerConfiguration.setThreshold(1);
- balancerConfiguration.setMaxSizeToMovePerIteration(10 * STORAGE_UNIT);
balancerConfiguration.setIterations(1);
- startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(500);
-
- // balancer should not have moved more size than the limit
- Assertions.assertFalse(
- containerBalancer.getSizeScheduledForMoveInLatestIteration() >
- 10 * STORAGE_UNIT);
-
- long size = containerBalancer.getMetrics()
- .getDataSizeMovedGBInLatestIteration();
- Assertions.assertTrue(size > 0);
- Assertions.assertFalse(size > 10);
- stopBalancer();
- }
-
- @Test
- public void targetDatanodeShouldNotAlreadyContainSelectedContainer()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- balancerConfiguration.setThreshold(10);
+ balancerConfiguration.setMaxSizeEnteringTarget(10 * STORAGE_UNIT);
balancerConfiguration.setMaxSizeToMovePerIteration(100 * STORAGE_UNIT);
balancerConfiguration.setMaxDatanodesPercentageToInvolvePerIteration(100);
- startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(1000);
-
- stopBalancer();
- Map<ContainerID, DatanodeDetails> map =
- containerBalancer.getContainerToTargetMap();
- for (Map.Entry<ContainerID, DatanodeDetails> entry : map.entrySet()) {
- ContainerID container = entry.getKey();
- DatanodeDetails target = entry.getValue();
- Assertions.assertTrue(cidToReplicasMap.get(container)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .noneMatch(target::equals));
- }
- }
+ balancerConfiguration.setMoveTimeout(Duration.ofMillis(1000));
- @Test
- public void containerMoveSelectionShouldFollowPlacementPolicy()
- throws IllegalContainerBalancerStateException, IOException,
- InvalidContainerBalancerConfigurationException, TimeoutException {
- balancerConfiguration.setThreshold(10);
- balancerConfiguration.setMaxSizeToMovePerIteration(50 * STORAGE_UNIT);
- balancerConfiguration.setMaxDatanodesPercentageToInvolvePerIteration(100);
- balancerConfiguration.setIterations(1);
startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(500);
-
- stopBalancer();
- Map<ContainerID, DatanodeDetails> containerFromSourceMap =
- containerBalancer.getContainerToSourceMap();
- Map<ContainerID, DatanodeDetails> containerToTargetMap =
- containerBalancer.getContainerToTargetMap();
-
- // for each move selection, check if {replicas - source + target}
- // satisfies placement policy
- for (Map.Entry<ContainerID, DatanodeDetails> entry :
- containerFromSourceMap.entrySet()) {
- ContainerID container = entry.getKey();
- DatanodeDetails source = entry.getValue();
-
- List<DatanodeDetails> replicas = cidToReplicasMap.get(container)
- .stream()
- .map(ContainerReplica::getDatanodeDetails)
- .collect(Collectors.toList());
- // remove source and add target
- replicas.remove(source);
- replicas.add(containerToTargetMap.get(container));
-
- ContainerInfo containerInfo = cidToInfoMap.get(container);
- ContainerPlacementStatus placementStatus =
- placementPolicy.validateContainerPlacement(replicas,
- containerInfo.getReplicationConfig().getRequiredNodes());
- Assertions.assertTrue(placementStatus.isPolicySatisfied());
- }
- }
-
- @Test
- public void targetDatanodeShouldBeInServiceHealthy()
- throws NodeNotFoundException, IllegalContainerBalancerStateException,
- IOException, InvalidContainerBalancerConfigurationException,
- TimeoutException {
- balancerConfiguration.setThreshold(10);
- balancerConfiguration.setMaxDatanodesPercentageToInvolvePerIteration(100);
- balancerConfiguration.setMaxSizeToMovePerIteration(50 * STORAGE_UNIT);
- balancerConfiguration.setMaxSizeEnteringTarget(50 * STORAGE_UNIT);
- balancerConfiguration.setIterations(1);
- startBalancer(balancerConfiguration);
-
- sleepWhileBalancing(500);
-
- stopBalancer();
- for (DatanodeDetails target : containerBalancer.getSelectedTargets()) {
- NodeStatus status = mockNodeManager.getNodeStatus(target);
- Assertions.assertSame(HddsProtos.NodeOperationalState.IN_SERVICE,
- status.getOperationalState());
- Assertions.assertTrue(status.isHealthy());
+ try {
+ containerBalancer.startBalancer(balancerConfiguration);
+ } catch (IllegalContainerBalancerStateException e) {
+ // start failed
+ Assertions.assertTrue(true);
Review Comment:
> Do we want to assert another variable here?
Here we are verifying that exception should occur when task is already
running, as behavior validation. Variable state is known to be running and
expectation that exception must be thrown.
--
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]
