You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "slfan1989 (via GitHub)" <gi...@apache.org> on 2026/06/06 07:17:57 UTC
[PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
slfan1989 opened a new pull request, #10450:
URL: https://github.com/apache/ozone/pull/10450
## What changes were proposed in this pull request?
This PR preserves the datanode order in the non-JSON output of the DiskBalancer status command.
The DiskBalancer status command executes requests in the order provided by the user, or in the order resolved by `--in-service-datanodes`. However, the non-JSON status output previously collected successful responses from `ConcurrentHashMap.values()`, which does not guarantee iteration order.
As a result, the displayed status rows could appear in a different order from the input datanode list. This PR updates the rendering path to build the status result list according to the ordered `successNodes` list instead.
The PR also updates the CLI unit test to verify that status output preserves the requested datanode order.
## What is the link to the Apache JIRA
HDDS-15491. [DiskBalancer] Preserve datanode order in status command output.
## How was this patch tested?
Add Junit Test.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #10450:
URL: https://github.com/apache/ozone/pull/10450#discussion_r3445367790
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerStatusSubcommand.java:
##########
Review Comment:
Thanks for the review. Updated. The requests are executed sequentially in `AbstractDiskBalancerSubCommand`, so `ConcurrentHashMap` is not required. Changed it to `LinkedHashMap` to keep the order-preserving behavior explicit.
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerStatusSubcommand.java:
##########
Review Comment:
Thanks for the review! Updated. The requests are executed sequentially in `AbstractDiskBalancerSubCommand`, so `ConcurrentHashMap` is not required. Changed it to `LinkedHashMap` to keep the order-preserving behavior explicit.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #10450:
URL: https://github.com/apache/ozone/pull/10450#discussion_r3445371576
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommands.java:
##########
@@ -513,21 +513,23 @@ public void testStatusDiskBalancerWithJson() throws Exception {
public void testStatusDiskBalancerWithMultipleNodes() throws Exception {
DiskBalancerStatusSubcommand cmd = new DiskBalancerStatusSubcommand();
- DatanodeDiskBalancerInfoProto statusProto1 = generateRandomStatusProto("host-1");
- DatanodeDiskBalancerInfoProto statusProto2 = generateRandomStatusProto("host-2");
+ DatanodeDiskBalancerInfoProto statusProto1 = generateRandomStatusProto("host-2");
+ DatanodeDiskBalancerInfoProto statusProto2 = generateRandomStatusProto("host-1");
when(mockProtocol.getDiskBalancerInfo())
.thenReturn(statusProto1, statusProto2);
try (DiskBalancerMocks mocks = setupAllMocks()) {
CommandLine c = new CommandLine(cmd);
- c.parseArgs("host-1", "host-2");
+ c.parseArgs("host-2", "host-1");
cmd.call();
String output = outContent.toString(DEFAULT_ENCODING);
- assertTrue(output.contains("host-1"));
- assertTrue(output.contains("host-2"));
+ int host2Index = output.indexOf("host-2");
+ int host1Index = output.indexOf("host-1");
+ assertTrue(host2Index >= 0);
+ assertTrue(host1Index > host2Index, output);
Review Comment:
Updated the order assertions to use AssertJ for readability, and applied the same style to the stdin order assertion as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #10450:
URL: https://github.com/apache/ozone/pull/10450#issuecomment-4756715366
> Thanks @slfan1989 for taking care of this. It's good to show output in the input order of dn hosts.
> Please add example output before the fix and after fix in Test Section of PR description.
@Gargi-jais11 Thanks for the review. I updated the PR test section with before/after example output to show that the non-JSON status output now follows the input datanode host order.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "sreejasahithi (via GitHub)" <gi...@apache.org>.
sreejasahithi commented on code in PR #10450:
URL: https://github.com/apache/ozone/pull/10450#discussion_r3385612164
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommands.java:
##########
@@ -513,21 +513,23 @@ public void testStatusDiskBalancerWithJson() throws Exception {
public void testStatusDiskBalancerWithMultipleNodes() throws Exception {
DiskBalancerStatusSubcommand cmd = new DiskBalancerStatusSubcommand();
- DatanodeDiskBalancerInfoProto statusProto1 = generateRandomStatusProto("host-1");
- DatanodeDiskBalancerInfoProto statusProto2 = generateRandomStatusProto("host-2");
+ DatanodeDiskBalancerInfoProto statusProto1 = generateRandomStatusProto("host-2");
+ DatanodeDiskBalancerInfoProto statusProto2 = generateRandomStatusProto("host-1");
when(mockProtocol.getDiskBalancerInfo())
.thenReturn(statusProto1, statusProto2);
try (DiskBalancerMocks mocks = setupAllMocks()) {
CommandLine c = new CommandLine(cmd);
- c.parseArgs("host-1", "host-2");
+ c.parseArgs("host-2", "host-1");
cmd.call();
String output = outContent.toString(DEFAULT_ENCODING);
- assertTrue(output.contains("host-1"));
- assertTrue(output.contains("host-2"));
+ int host2Index = output.indexOf("host-2");
+ int host1Index = output.indexOf("host-1");
+ assertTrue(host2Index >= 0);
+ assertTrue(host1Index > host2Index, output);
Review Comment:
This file already uses AssertJ (assertThat).
For readability, consider
assertThat(host2Index).isGreaterThanOrEqualTo(0);
assertThat(host1Index).isGreaterThan(host2Index);
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerStatusSubcommand.java:
##########
Review Comment:
Not related to your change, but is ConcurrentHashMap needed here?
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommands.java:
##########
Review Comment:
`testStatusDiskBalancerWithMultipleNodes` now asserts output order, but `testStatusDiskBalancerWithStdin` still only checks contains for host-1/host-2 even though stdin order is host-1 then host-2.
Consider applying the same order assertion here so stdin input path is covered too.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on code in PR #10450:
URL: https://github.com/apache/ozone/pull/10450#discussion_r3445369714
##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDiskBalancerSubCommands.java:
##########
Review Comment:
Updated `testStatusDiskBalancerWithStdin` to assert that the output preserves the stdin input order as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 merged PR #10450:
URL: https://github.com/apache/ozone/pull/10450
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15491. [DiskBalancer] Preserve datanode order in status command output. [ozone]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #10450:
URL: https://github.com/apache/ozone/pull/10450#issuecomment-4794807945
> Thanks for updating the patch @slfan1989
> LGTM
@Gargi-jais11 @sreejasahithi Thank you very much for helping review the code!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org