Copilot commented on code in PR #10755:
URL: https://github.com/apache/ozone/pull/10755#discussion_r3595376205
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerSubCommandUtil.java:
##########
@@ -95,31 +159,33 @@ public static Map<String, String>
getAllOperableNodesClientRpcAddress(
if (node.getNodeStates(0).equals(HddsProtos.NodeState.DEAD)) {
continue;
}
- Port port = details.getPort(Port.Name.CLIENT_RPC);
- if (port != null) {
- String address = details.getIpAddress() + ":" + port.getValue();
- // Format the display string: "hostname (ip:port)" or "ip:port"
- String hostname = details.getHostName();
- String display = (hostname != null && !hostname.isEmpty()
- && !hostname.equals(details.getIpAddress())) ? hostname + " (" +
address + ")"
- : address;
- addressToDisplay.put(address, display);
- } else {
- System.out.printf("host: %s(%s) %s port not found%n",
- details.getHostName(), details.getIpAddress(),
- Port.Name.CLIENT_RPC.name());
+ try {
+ String address = getClientRpcAddress(details);
+ addressToDisplay.put(address, getDatanodeHostAndIp(node.getNodeID()));
+ } catch (IOException e) {
+ System.out.println(e.getMessage());
}
Review Comment:
getAllOperableNodesClientRpcAddress writes resolution errors to stdout. In
JSON mode (eg `--json --in-service-datanodes`), any stdout output before the
JSON payload will corrupt the JSON output. Send these messages to stderr
instead.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/AbstractDiskBalancerSubCommand.java:
##########
@@ -154,11 +184,51 @@ private List<String> getTargetDatanodes() {
if (options.isInServiceDatanodes()) {
return getAllInServiceDatanodes();
} else {
- datanodeDisplayNames = null; // Non-batch: use user input as-is, no SCM
for formatting
- return options.getDatanodes();
+ return resolveExplicitDatanodeAddresses(options.getDatanodes());
}
}
+ /**
+ * Resolves datanode UUIDs to CLIENT_RPC addresses via SCM.
+ * Hostname and host:port arguments are passed through unchanged.
+ */
+ private List<String> resolveExplicitDatanodeAddresses(List<String> nodeArgs)
{
+ if
(nodeArgs.stream().noneMatch(DiskBalancerSubCommandUtil::isDatanodeUuid)) {
+ datanodeDisplayNames = null;
+ return nodeArgs;
+ }
+
+ Map<String, String> displayNames = new LinkedHashMap<>();
+ List<String> resolvedAddresses = new ArrayList<>();
+ try (ScmClient scmClient = new ContainerOperationClient(new
OzoneConfiguration())) {
+ for (String nodeArg : nodeArgs) {
+ try {
+ DiskBalancerSubCommandUtil.DatanodeTarget target =
+ DiskBalancerSubCommandUtil.resolveDatanodeTarget(scmClient,
nodeArg);
+ resolvedAddresses.add(target.getClientRpcAddress());
+ displayNames.put(target.getClientRpcAddress(),
target.getDisplayName());
+ } catch (IOException e) {
+ addResolutionFailure(nodeArg, e.getMessage());
+ }
+ }
+ } catch (IOException e) {
+ System.err.printf("Error resolving datanode address(es). %n%s%n",
e.getMessage());
+ return null;
+ }
Review Comment:
If SCM client creation fails (eg SCM down) while resolving UUIDs, this
method returns null and the whole command aborts, even though non-UUID
host/host:port arguments could still be executed. This also prevents per-UUID
failures from being reported in JSON mode. Prefer falling back to executing
non-UUID arguments and recording UUID resolution failures when SCM cannot be
contacted.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DatanodeParameters.java:
##########
@@ -43,10 +43,11 @@ public class DatanodeParameters extends ItemsFromStdin {
" # From file having list of dns to balance",
" ozone admin datanode diskbalancer report - < datanode-lists.txt",
"Port is optional and defaults to 19864 (CLIENT_RPC port).",
- "Address examples: 'DN-1', 'DN-1:19864', '192.168.1.10'."
+ "Address examples: 'DN-1', 'DN-1:19864', '192.168.1.10',",
+ " 'a3b63511-bdf8-4fa1-8ab6-d19c0e806f84' (datanode UUID)."
},
arity = "0..*",
- paramLabel = "<datanode address>")
+ paramLabel = "<datanode address or id>")
Review Comment:
The CLI help text currently prints a trailing comma at the end of the
"Address examples" line (",") which looks like a formatting artifact in the
rendered usage output. Consider removing the comma and ending the sentence
cleanly.
--
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]