ashishkumar50 commented on code in PR #10812:
URL: https://github.com/apache/ozone/pull/10812#discussion_r3629056214
##########
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:
Good to extract this to shared helper, else if balancer later changes to use
ceil, max or something else, this validation will become inconsistent.
##########
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);
+ if (maxDatanodesToInvolve < 2) {
+ throw new InvalidContainerBalancerConfigurationException(String.format(
+ "max-datanodes-percentage-to-involve-per-iteration=%d allows at most
"
+ + "%d datanode(s) per iteration with %d eligible datanode(s), "
+ + "but at least 2 are required for a source and target datanode "
+ + "pair.",
+ conf.getMaxDatanodesPercentageToInvolvePerIteration(),
+ maxDatanodesToInvolve, eligibleCount));
+ }
+ }
+
+ /**
+ * Counts healthy, in-service datanodes that can participate in balancing
after
+ * applying include/exclude datanode configuration.
+ */
+ int countEligibleDatanodes(ContainerBalancerConfiguration conf) {
Review Comment:
nit: `private`
##########
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:
`(int)(0.20×eligibleCount)≥2==>0.20×eligibleCount≥2==>eligibleCount≥10`
So a cluster with 6 healthy in-service datanodes passes Check 1 (6 ≥ 2) but
fails Check 2, because (int)(0.20 × 6) = 1 < 2.
Any cluster with fewer than 10 eligible datanodes will fail in Check 2 and
fails to start, even though it has well more than 2 nodes.
If at least 2 nodes are available we should allow balancer to start and it
should work.
--
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]
