sreejasahithi commented on code in PR #10812:
URL: https://github.com/apache/ozone/pull/10812#discussion_r3643037495


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java:
##########
@@ -510,6 +541,133 @@ private void 
validateConfiguration(ContainerBalancerConfiguration conf)
 
     validateNodeList(conf.getIncludeNodes(), "included");
     validateNodeList(conf.getExcludeNodes(), "excluded");
+    validateIncludeExcludeLists(conf);
+    validateIncludeContainersExist(conf);
+    validateEligibleDatanodePool(conf);
+  }
+
+  /**
+   * Rejects include lists that are fully covered by the corresponding exclude
+   * lists, which would leave no datanodes or containers to balance.
+   */
+  private void validateIncludeExcludeLists(ContainerBalancerConfiguration conf)
+      throws InvalidContainerBalancerConfigurationException {
+    Set<String> includeNodes = conf.getIncludeNodes();
+    Set<String> excludeNodes = conf.getExcludeNodes();
+    if (!includeNodes.isEmpty() && !excludeNodes.isEmpty()) {
+      boolean allIncludedNodesExcluded = true;
+      for (String includedNode : includeNodes) {
+        if (!isIncludedNodeExcluded(includedNode, excludeNodes)) {
+          allIncludedNodesExcluded = false;
+          break;
+        }
+      }
+      if (allIncludedNodesExcluded) {
+        throw new InvalidContainerBalancerConfigurationException(
+            "include-datanodes is a subset of exclude-datanodes, no datanode 
can participate in balancing.");
+      }
+    }
+
+    Set<ContainerID> includeContainers = conf.getIncludeContainers();
+    Set<ContainerID> excludeContainers = conf.getExcludeContainers();
+    if (!includeContainers.isEmpty() && 
excludeContainers.containsAll(includeContainers)) {
+      throw new InvalidContainerBalancerConfigurationException(
+          "include-containers is a subset of exclude-containers, no container 
can be selected for balancing.");
+    }
+  }
+
+  private boolean isIncludedNodeExcluded(String includedNode, Set<String> 
excludeNodes) {
+    if (excludeNodes.contains(includedNode)) {
+      return true;
+    }
+    for (DatanodeDetails dn : 
scm.getScmNodeManager().getNodesByAddress(includedNode)) {
+      if (excludeNodes.contains(dn.getHostName()) || 
excludeNodes.contains(dn.getIpAddress())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Rejects non-empty include-containers lists when any listed container ID
+   * does not exist in SCM.
+   */
+  private void validateIncludeContainersExist(ContainerBalancerConfiguration 
conf)
+      throws InvalidContainerBalancerConfigurationException {
+    Set<ContainerID> includeContainers = conf.getIncludeContainers();
+    if (includeContainers.isEmpty()) {
+      return;
+    }
+
+    ContainerManager containerManager = scm.getContainerManager();
+    List<ContainerID> missingContainers = new ArrayList<>();
+    for (ContainerID containerID : includeContainers) {
+      try {
+        containerManager.getContainer(containerID);
+      } catch (ContainerNotFoundException e) {
+        missingContainers.add(containerID);
+      }
+    }
+
+    if (!missingContainers.isEmpty()) {
+      throw new InvalidContainerBalancerConfigurationException(
+          "Container Balancer cannot start: included container ID(s) " + 
missingContainers
+              + " do not exist in SCM.");
+    }
+  }
+
+  /**
+   * Validates that enough healthy, in-service datanodes are eligible and that
+   * {@link 
ContainerBalancerConfiguration#getMaxDatanodesRatioToInvolvePerIteration()}
+   * allows at least one source and one target datanode per iteration.
+   */
+  private void validateEligibleDatanodePool(ContainerBalancerConfiguration 
conf)
+      throws InvalidContainerBalancerConfigurationException {
+    int eligibleCount = countEligibleDatanodes(conf);
+    if (eligibleCount < 2) {
+      throw new InvalidContainerBalancerConfigurationException(String.format(
+          "Container Balancer requires at least 2 eligible datanodes but only "
+              + "%d is available.", eligibleCount));
+    }
+    int maxDatanodesToInvolve = (int) 
(conf.getMaxDatanodesRatioToInvolvePerIteration() * eligibleCount);

Review Comment:
   The behavior when these checks are removed at startup. With 6 eligible 
datanodes and the default 20%, (int)(0.20 × 6) = 1, so maxDatanodesToInvolve = 
1.
   
   The balancer still starts, but in ContainerBalancerTask.doIteration() the 
first call to adaptWhenNearingIterationLimits() fires immediately (count=0, 
0+1==1), which calls resetPotentialTargets(selectedTargets) with an empty set. 
That clears all potential targets before any source/target pairing happens so 
no moves are scheduled. The iteration ends with CAN_NOT_BALANCE_ANY_MORE and 
the balancer stops with "No more eligible container moves were found".
   
   Since balancing requires at least one source and one target (2 datanodes), a 
cap of 1 makes balancing impossible under the configured limit. Rejecting this 
at startup with an explicit config error is better than starting and 
immediately stopping.
   
   For clusters with fewer than 10 eligible datanodes, the user needs to raise 
--max-datanodes-percentage-to-involve-per-iteration. the percentage is a hard 
cap, and if the cap is below the minimum needed for a source/target pair the 
config is invalid for that cluster size.



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

Reply via email to