JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686450821
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
Review comment:
can you handle this? these can also be initialized at the constructor
function and reused with `clear`
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -200,6 +228,8 @@ private boolean initializeIteration() {
// find over and under utilized nodes
for (DatanodeUsageInfo datanodeUsageInfo : datanodeUsageInfos) {
double utilization = calculateUtilization(datanodeUsageInfo);
+ LOG.info("Utilization for node {} is {}",
+ datanodeUsageInfo.getDatanodeDetails().getUuidString(), utilization);
if (utilization > upperLimit) {
overUtilizedNodes.add(datanodeUsageInfo);
countDatanodesToBalance += 1;
Review comment:
can you handle this? by the way, `canSizeLeaveSource`,
`getClusterAvgUtilisation` seem not used now, we can remove them
##########
File path:
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -219,7 +505,7 @@ private double createNodesInCluster() {
for (double utilization : nodeUtilizations) {
// select a random index from 0 to capacities.length
- int index = ThreadLocalRandom.current().nextInt(0, capacities.length);
Review comment:
`createNodesInCluster` seems not used
##########
File path:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws
Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
yes , that is a temporary implementation before balancer is fully
completed and it is just for testing cli command, so it is may not Compatible
with current balancer logic. could you also fix it 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: [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]
