sreejasahithi commented on code in PR #11256: URL: https://github.com/apache/ozone/pull/11256#discussion_r4060289328
########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerAssessmentSubcommand.java: ########## Review Comment: The command applies -t or config default, but the printed report never shows it. For better understanding, add something like: Threshold: 10.0% Optionally Upper limit / Lower limit (values already exist on the snapshot) Without this, users cannot verify why nodes were classified as source/target without doing mental math. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerAssessmentSubcommand.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.cli; + +import static org.apache.hadoop.util.StringUtils.byteDesc; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DatanodeUsageInfoProto; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerClusterAnalyzer; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerClusterSnapshot; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerConfiguration; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** Reports cluster imbalance without starting the container balancer. */ +@Command( + name = "assessment", + description = "Report cluster imbalance before starting ContainerBalancer", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class ContainerBalancerAssessmentSubcommand extends ScmSubcommand { + + @Option(names = {"-t", "--threshold"}, + description = "Percentage deviation from average utilization of " + + "the cluster after which a datanode is considered over- or under-utilized. " + + "The value should be in the range [0.0, 100.0), with a default of 10 " + + "(specify '10' for 10%%).") + private Optional<Double> threshold; + + @Option(names = {"-n", "--limit"}, + description = "Maximum number of over- and under-utilized datanodes to list in the report. Default: 5.") + private int nodeLimit = 5; + + @Option(names = {"--include-datanodes"}, + description = "A list of Datanode hostnames or ip addresses separated by commas. " + + "Only the Datanodes specified in this list are included in the assessment.") + private Optional<String> includeNodes; + + @Option(names = {"--exclude-datanodes"}, + description = "A list of Datanode hostnames or ip addresses separated by commas. " + + "The Datanodes specified in this list are excluded from the assessment.") + private Optional<String> excludeNodes; + + @Override + public void execute(ScmClient scmClient) throws IOException { + ContainerBalancerConfiguration config = + getOzoneConf().getObject(ContainerBalancerConfiguration.class); + + if (nodeLimit < 1) { + throw new IllegalArgumentException("limit must be at least 1."); + } + + double thresholdRatio = threshold + .map(t -> { + if (t < 0d || t >= 100d) { + throw new IllegalArgumentException( + "Threshold must be a percentage in the range [0.0, 100.0)."); + } + return t / 100.0; + }) + .orElseGet(config::getThresholdAsRatio); + + Set<String> include = includeNodes + .map(ContainerBalancerAssessmentSubcommand::parseNodeList) + .orElseGet(config::getIncludeNodes); + Set<String> exclude = excludeNodes + .map(ContainerBalancerAssessmentSubcommand::parseNodeList) + .orElseGet(config::getExcludeNodes); + + List<DatanodeUsageInfoProto> nodes = + scmClient.getDatanodeUsageInfo(true, Integer.MAX_VALUE); + + ContainerBalancerClusterSnapshot snapshot = + ContainerBalancerClusterAnalyzer.analyze(nodes, thresholdRatio, include, exclude, nodeLimit); + + printReport(snapshot, nodeLimit); + } + + private static void printReport(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + System.out.println("CLUSTER BALANCE ASSESSMENT"); + + if (snapshot.getTotalEligibleDatanodes() == 0) { + System.out.println(getSummaryPrettyString(snapshot)); + System.out.println(); + System.out.println("No eligible datanodes found for assessment."); + return; + } + + System.out.println(getSummaryPrettyString(snapshot)); + System.out.println(); + System.out.println(getSourceNodesPrettyString(snapshot, nodeLimit)); + System.out.println(); + System.out.println(getTargetNodesPrettyString(snapshot, nodeLimit)); + System.out.println(); + System.out.println(getMovementSummaryPrettyString(snapshot)); + } + + private static String getSummaryPrettyString(ContainerBalancerClusterSnapshot snapshot) { + if (snapshot.getTotalEligibleDatanodes() == 0) { + return String.format("%-50s %s%n" + + "%-50s %s%n", "Key", "Value", + "Eligible datanodes", "0 datanodes"); + } + return String.format("%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n", "Key", "Value", + "Drift", String.format(Locale.US, + "%.1f%% (max utilization %.1f%% - min utilization %.1f%%)", + snapshot.getImbalance() * 100, + snapshot.getMaxUtilization() * 100, + snapshot.getMinUtilization() * 100), + "Mean Utilization", formatPercent(snapshot.getClusterAvgUtilization()), + "Eligible datanodes", snapshot.getTotalEligibleDatanodes() + " datanode" + + (snapshot.getTotalEligibleDatanodes() == 1 ? "" : "s"), + "Category", formatCategory(snapshot)); + } + + private static String getSourceNodesPrettyString(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + StringBuilder builder = new StringBuilder(); + builder.append("Source Nodes (over-utilized):").append(System.lineSeparator()) + .append(String.format("%-50s %s%n", "Datanodes above threshold", snapshot.getSourceCount())); + appendNodeList(builder, snapshot.getTopSourceNodes(), snapshot.getClusterAvgUtilization(), true, nodeLimit); + return builder.toString(); + } + + private static String getTargetNodesPrettyString(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + StringBuilder builder = new StringBuilder(); + builder.append("Target Nodes (under-utilized):").append(System.lineSeparator()) + .append(String.format("%-50s %s%n", "Datanodes below threshold", snapshot.getTargetCount())); + appendNodeList(builder, snapshot.getBottomTargetNodes(), snapshot.getClusterAvgUtilization(), false, nodeLimit); + return builder.toString(); + } + + private static String getMovementSummaryPrettyString(ContainerBalancerClusterSnapshot snapshot) { + double movementRatio = snapshot.getClusterCapacityBytes() == 0 ? 0 + : (double) snapshot.getBytesToMove() / snapshot.getClusterCapacityBytes(); + return String.format("Movement Summary:%n" + + "%-50s %s%n" + + "%-50s %s%n", "Total bytes to move", byteDesc(snapshot.getBytesToMove()), + "Movement ratio", + String.format(Locale.US, "%.1f%% of cluster capacity", movementRatio * 100)); + } + + private static void appendNodeList(StringBuilder builder, + List<ContainerBalancerClusterSnapshot.NodeUtilization> nodes, + double clusterAvgUtilization, boolean aboveMean, int nodeLimit) { + if (nodes.isEmpty()) { + return; + } + builder.append(System.lineSeparator()) + .append(aboveMean ? "Top " + nodeLimit + ":" : "Bottom " + nodeLimit + ":") Review Comment: The header always uses nodeLimit (default 5), even when fewer nodes are listed (your output shows "Top 5:" with 3 rows). Use the actual list size when list size is < limit ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerAssessmentSubcommand.java: ########## @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.cli; + +import static org.apache.hadoop.util.StringUtils.byteDesc; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DatanodeUsageInfoProto; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerClusterAnalyzer; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerClusterSnapshot; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerConfiguration; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** Reports cluster imbalance without starting the container balancer. */ +@Command( + name = "assessment", + description = "Report cluster imbalance before starting ContainerBalancer", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class ContainerBalancerAssessmentSubcommand extends ScmSubcommand { + + @Option(names = {"-t", "--threshold"}, + description = "Percentage deviation from average utilization of " + + "the cluster after which a datanode is considered over- or under-utilized. " + + "The value should be in the range [0.0, 100.0), with a default of 10 " + + "(specify '10' for 10%%).") + private Optional<Double> threshold; + + @Option(names = {"-n", "--limit"}, + description = "Maximum number of over- and under-utilized datanodes to list in the report. Default: 5.") + private int nodeLimit = 5; + + @Option(names = {"--include-datanodes"}, + description = "A list of Datanode hostnames or ip addresses separated by commas. " + + "Only the Datanodes specified in this list are included in the assessment.") + private Optional<String> includeNodes; + + @Option(names = {"--exclude-datanodes"}, + description = "A list of Datanode hostnames or ip addresses separated by commas. " + + "The Datanodes specified in this list are excluded from the assessment.") + private Optional<String> excludeNodes; + + @Override + public void execute(ScmClient scmClient) throws IOException { + ContainerBalancerConfiguration config = + getOzoneConf().getObject(ContainerBalancerConfiguration.class); + + if (nodeLimit < 1) { + throw new IllegalArgumentException("limit must be at least 1."); + } + + double thresholdRatio = threshold + .map(t -> { + if (t < 0d || t >= 100d) { + throw new IllegalArgumentException( + "Threshold must be a percentage in the range [0.0, 100.0)."); + } + return t / 100.0; + }) + .orElseGet(config::getThresholdAsRatio); + + Set<String> include = includeNodes + .map(ContainerBalancerAssessmentSubcommand::parseNodeList) + .orElseGet(config::getIncludeNodes); + Set<String> exclude = excludeNodes + .map(ContainerBalancerAssessmentSubcommand::parseNodeList) + .orElseGet(config::getExcludeNodes); + + List<DatanodeUsageInfoProto> nodes = + scmClient.getDatanodeUsageInfo(true, Integer.MAX_VALUE); + + ContainerBalancerClusterSnapshot snapshot = + ContainerBalancerClusterAnalyzer.analyze(nodes, thresholdRatio, include, exclude, nodeLimit); + + printReport(snapshot, nodeLimit); + } + + private static void printReport(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + System.out.println("CLUSTER BALANCE ASSESSMENT"); + + if (snapshot.getTotalEligibleDatanodes() == 0) { + System.out.println(getSummaryPrettyString(snapshot)); + System.out.println(); + System.out.println("No eligible datanodes found for assessment."); + return; + } + + System.out.println(getSummaryPrettyString(snapshot)); + System.out.println(); + System.out.println(getSourceNodesPrettyString(snapshot, nodeLimit)); + System.out.println(); + System.out.println(getTargetNodesPrettyString(snapshot, nodeLimit)); + System.out.println(); + System.out.println(getMovementSummaryPrettyString(snapshot)); + } + + private static String getSummaryPrettyString(ContainerBalancerClusterSnapshot snapshot) { + if (snapshot.getTotalEligibleDatanodes() == 0) { + return String.format("%-50s %s%n" + + "%-50s %s%n", "Key", "Value", + "Eligible datanodes", "0 datanodes"); + } + return String.format("%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n" + + "%-50s %s%n", "Key", "Value", + "Drift", String.format(Locale.US, + "%.1f%% (max utilization %.1f%% - min utilization %.1f%%)", + snapshot.getImbalance() * 100, + snapshot.getMaxUtilization() * 100, + snapshot.getMinUtilization() * 100), + "Mean Utilization", formatPercent(snapshot.getClusterAvgUtilization()), + "Eligible datanodes", snapshot.getTotalEligibleDatanodes() + " datanode" + + (snapshot.getTotalEligibleDatanodes() == 1 ? "" : "s"), + "Category", formatCategory(snapshot)); + } + + private static String getSourceNodesPrettyString(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + StringBuilder builder = new StringBuilder(); + builder.append("Source Nodes (over-utilized):").append(System.lineSeparator()) + .append(String.format("%-50s %s%n", "Datanodes above threshold", snapshot.getSourceCount())); + appendNodeList(builder, snapshot.getTopSourceNodes(), snapshot.getClusterAvgUtilization(), true, nodeLimit); + return builder.toString(); + } + + private static String getTargetNodesPrettyString(ContainerBalancerClusterSnapshot snapshot, int nodeLimit) { + StringBuilder builder = new StringBuilder(); + builder.append("Target Nodes (under-utilized):").append(System.lineSeparator()) + .append(String.format("%-50s %s%n", "Datanodes below threshold", snapshot.getTargetCount())); + appendNodeList(builder, snapshot.getBottomTargetNodes(), snapshot.getClusterAvgUtilization(), false, nodeLimit); + return builder.toString(); + } + + private static String getMovementSummaryPrettyString(ContainerBalancerClusterSnapshot snapshot) { + double movementRatio = snapshot.getClusterCapacityBytes() == 0 ? 0 + : (double) snapshot.getBytesToMove() / snapshot.getClusterCapacityBytes(); + return String.format("Movement Summary:%n" + + "%-50s %s%n" + + "%-50s %s%n", "Total bytes to move", byteDesc(snapshot.getBytesToMove()), + "Movement ratio", + String.format(Locale.US, "%.1f%% of cluster capacity", movementRatio * 100)); + } + + private static void appendNodeList(StringBuilder builder, + List<ContainerBalancerClusterSnapshot.NodeUtilization> nodes, + double clusterAvgUtilization, boolean aboveMean, int nodeLimit) { + if (nodes.isEmpty()) { + return; Review Comment: When there are no over- or under-utilized datanodes (Datanodes above threshold: 0 / Datanodes below threshold: 0), the command still prints the "Source Nodes" and "Target Nodes" section headers, but no node list appears underneath only a blank line before the next section. That is correct behaviour, but it can look like output was cut off. Consider adding an explicit line such as (none) when the count is 0, so users can tell the command finished successfully and simply found no matching nodes. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerAssessmentSubcommand.java: ########## Review Comment: we can also add a --json option. -- 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]
