sravani-revuri commented on code in PR #11255: URL: https://github.com/apache/ozone/pull/11255#discussion_r4070843271
########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerConfigOptions.java: ########## @@ -0,0 +1,175 @@ +/* + * 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 java.time.Duration; +import java.util.Arrays; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerAdvisor; +import org.apache.hadoop.ozone.OzoneConsts; +import picocli.CommandLine.Option; + +/** + * Shared Picocli options for container balancer commands. + */ +public class ContainerBalancerConfigOptions { + + @Option(names = {"-t", "--threshold"}, + description = "Percentage deviation from average utilization of " + + "the cluster after which a datanode will be rebalanced. 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 = {"-d", "--max-datanodes-percentage-to-involve-per-iteration"}, + description = "Max percentage of healthy, in service datanodes " + + "that can be involved in balancing in one iteration. The value " + + "should be in the range [0,100]. When omitted on dry-run, each profile uses its default preset. " + Review Comment: here the range says [0,100] which means it is inclusive of 0 but the validateMaxDatanodesPercentageToInvolvePerIteration thows error when it is 0. can this be changed to (0,100] ? ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerDryRunSubcommand.java: ########## @@ -0,0 +1,166 @@ +/* + * 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.List; +import java.util.Locale; +import java.util.Optional; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +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.ContainerBalancerAdvisor; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerEstimation; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerProfile; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * Estimates container balancer bytes to move, per iteration bytes, estimated iterations, estimated duration + * without starting the balancer. + */ +@Command( + name = "dry-run", + description = "Estimate container balancer bytes to move, iterations, per iteration bytes " + + "and upper-bound duration without starting it. Balancer limits and profile presets are read from " + + "the local Ozone configuration (including ozone-site.xml), datanode usage is fetched from SCM.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class ContainerBalancerDryRunSubcommand extends ScmSubcommand { + + private static final double PLANNING_ITERATION_BUFFER = 1.3d; + + @CommandLine.Mixin + private ContainerBalancerConfigOptions configOptions; + + @CommandLine.ArgGroup(exclusive = true, multiplicity = "0..1") + private ProfileSelection profileSelection; + + @Override + public void execute(ScmClient scmClient) throws IOException { + + List<DatanodeUsageInfoProto> nodes = scmClient.getDatanodeUsageInfo(true, Integer.MAX_VALUE); + if (nodes == null || nodes.isEmpty()) { + throw new IOException("No datanode usage information available from SCM."); + } + + OzoneConfiguration conf = getOzoneConf(); + ContainerBalancerAdvisor.AdvisorRequest request = buildRequest(nodes); + List<ContainerBalancerEstimation> estimations; + try { + estimations = ContainerBalancerAdvisor.estimateDryRun(conf, request); + } catch (IllegalArgumentException e) { + throw new IOException(e.getMessage(), e); + } + + boolean anySucceeded = false; + for (ContainerBalancerEstimation result : estimations) { + out().printf("Profile: %s%n", result.getProfile().name()); + printBasedOn(result); + if (result.succeeded()) { + anySucceeded = true; + printEstimation(result); + } else { + out().printf(" Estimation failed: %s%n%n", result.getFailureMessage()); + } + } + if (!anySucceeded) { + throw new IOException(estimations.get(0).getFailureMessage()); + } + } + + private ContainerBalancerAdvisor.AdvisorRequest buildRequest(List<DatanodeUsageInfoProto> nodes) throws IOException { + ContainerBalancerAdvisor.AdvisorRequest request = new ContainerBalancerAdvisor.AdvisorRequest().setNodes(nodes); + configOptions.applyToDryRunRequest(request); + + if (profileSelection != null) { + if (profileSelection.allProfiles) { + request.setAllProfiles(true); + } else { + request.setProfile(parseProfile(profileSelection.profileName.get())); + } + } + return request; + } + + private static ContainerBalancerProfile parseProfile(String name) throws IOException { + try { + return ContainerBalancerProfile.valueOf(name.trim().toUpperCase(Locale.ENGLISH)); + } catch (IllegalArgumentException e) { + throw new IOException("Invalid profile: " + name + ". Expected SLOW, MEDIUM, or FAST."); + } + } + + private void printBasedOn(ContainerBalancerEstimation estimation) { + long moveTimeoutMinutes = Math.round(estimation.getMoveTimeoutMillis() / 60000d); + long balancingIntervalMinutes = Math.round(estimation.getBalancingIntervalMillis() / 60000d); + out().println(" Based on:"); + out().printf(Locale.ENGLISH, " Datanode involvement: %d%%%n", + estimation.getMaxDatanodesPercentage()); + out().printf(" Max entering target: %s / node%n", byteDesc(estimation.getMaxSizeEnteringTarget())); + out().printf(" Max leaving source: %s / node%n", byteDesc(estimation.getMaxSizeLeavingSource())); + out().printf(" Max per iteration: %s%n", byteDesc(estimation.getMaxSizeToMovePerIteration())); + out().printf(" Move timeout: %d min%n", moveTimeoutMinutes); + out().printf(" Balancing interval: %d min%n", balancingIntervalMinutes); + } + + private void printEstimation(ContainerBalancerEstimation estimation) { Review Comment: Would it be better to print the threshold in the output too? Dry-run uses -t (or the config default) to give the estimated bytes to move, but the report doesn’t show it. similar to the assessment and recommendation command. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerDryRunSubcommand.java: ########## @@ -0,0 +1,166 @@ +/* + * 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.List; +import java.util.Locale; +import java.util.Optional; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +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.ContainerBalancerAdvisor; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerEstimation; +import org.apache.hadoop.hdds.scm.container.balancer.ContainerBalancerProfile; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * Estimates container balancer bytes to move, per iteration bytes, estimated iterations, estimated duration + * without starting the balancer. + */ +@Command( + name = "dry-run", + description = "Estimate container balancer bytes to move, iterations, per iteration bytes " + + "and upper-bound duration without starting it. Balancer limits and profile presets are read from " + + "the local Ozone configuration (including ozone-site.xml), datanode usage is fetched from SCM.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class ContainerBalancerDryRunSubcommand extends ScmSubcommand { + + private static final double PLANNING_ITERATION_BUFFER = 1.3d; + + @CommandLine.Mixin + private ContainerBalancerConfigOptions configOptions; + + @CommandLine.ArgGroup(exclusive = true, multiplicity = "0..1") + private ProfileSelection profileSelection; + + @Override + public void execute(ScmClient scmClient) throws IOException { + + List<DatanodeUsageInfoProto> nodes = scmClient.getDatanodeUsageInfo(true, Integer.MAX_VALUE); + if (nodes == null || nodes.isEmpty()) { + throw new IOException("No datanode usage information available from SCM."); + } + + OzoneConfiguration conf = getOzoneConf(); + ContainerBalancerAdvisor.AdvisorRequest request = buildRequest(nodes); + List<ContainerBalancerEstimation> estimations; + try { + estimations = ContainerBalancerAdvisor.estimateDryRun(conf, request); + } catch (IllegalArgumentException e) { + throw new IOException(e.getMessage(), e); + } + + boolean anySucceeded = false; + for (ContainerBalancerEstimation result : estimations) { Review Comment: when no profile is specified would it be better to specify that we're going with default? something like : ``` No profile specified using the default. Profile: MEDIUM ... ``` -- 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]
