Skip to content

Conversation

@Gargi-jais11
Copy link
Contributor

@Gargi-jais11 Gargi-jais11 commented Dec 19, 2025

What changes were proposed in this pull request?

There are few issues with usability and readability of diskbalancer cli output:

Firstly, used set instead of list to store dnHostName for cli output so that it doesn't display duplicate hostnames if user give any diskbalancer command for same hostname twice like below:

bash-5.1$ ozone admin datanode diskbalancer status ozone-datanode-1 ozone-datanode-3 ozone-datanode-1 --json
[ {
  "datanode" : "ozone-datanode-1.ozone_default",
  "action" : "status",
  "status" : "success",
  "serviceStatus" : "RUNNING",
  "threshold" : 10.0,
  "bandwidthInMB" : 10,
  "threads" : 5,
  "stopAfterDiskEven" : false,
  "successMove" : 0,
  "failureMove" : 0,
  "bytesMovedMB" : 0,
  "estBytesToMoveMB" : 0,
  "estTimeLeftMin" : 0
}, {
  "datanode" : "ozone-datanode-3.ozone_default",
  "action" : "status",
  "status" : "success",
  "serviceStatus" : "RUNNING",
  "threshold" : 10.0,
  "bandwidthInMB" : 10,
  "threads" : 5,
  "stopAfterDiskEven" : false,
  "successMove" : 0,
  "failureMove" : 0,
  "bytesMovedMB" : 0,
  "estBytesToMoveMB" : 0,
  "estTimeLeftMin" : 0
}, {
"datanode" : "ozone-datanode-1.ozone_default",
  "action" : "status",
  "status" : "success",
  "serviceStatus" : "RUNNING",
  "threshold" : 10.0,
  "bandwidthInMB" : 10,
  "threads" : 5,
  "stopAfterDiskEven" : false,
  "successMove" : 0,
  "failureMove" : 0,
  "bytesMovedMB" : 0,
  "estBytesToMoveMB" : 0,
  "estTimeLeftMin" : 0
} ]

Secondly, Instead of showing ip-address in json cli output show dnHostName for better clarity.

bash-5.1$ ozone admin datanode diskbalancer stop --in-service-datanodes --json
[ {
  "datanode" : "172.18.0.5:19864",
  "action" : "stop",
  "status" : "success"
}, {
  "datanode" : "172.18.0.9:19864",
  "action" : "stop",
  "status" : "success"
}, {
  "datanode" : "172.18.0.10:19864",
  "action" : "stop",
  "status" : "success"
}, {
  "datanode" : "172.18.0.8:19864",
  "action" : "stop",
  "status" : "success"
}]

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14195

How was this patch tested?

Added new test cases to TestDiskBalancerProtocolServer and update TestDiskBalancerSubCommands, TestDiskBalancer.
Tested locally:

@Gargi-jais11 Gargi-jais11 marked this pull request as ready for review December 19, 2025 08:13
@Gargi-jais11 Gargi-jais11 changed the base branch from HDDS-5713 to master January 8, 2026 04:17
@adoroszlai adoroszlai changed the title HDDS-14195. DiskBalancer should not send another start operation on a DN with an active DiskBalancer run HDDS-14195. Do not send DiskBalancer start command to DN with active run Jan 15, 2026
Map<String, Object> configMap = getConfigurationMap();
if (configMap != null && !configMap.isEmpty()) {
result.put("configuration", configMap);
if (status.getRunningStatus() == DiskBalancerRunningStatus.RUNNING ||
Copy link
Contributor

@ChenSammi ChenSammi Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not do the filtering.

It's ok to send the start command again to a DN which has disk balancer running, and doesn't response with "*** is already running". It's called API idempotent.

Further more, filter DN at client side is inaccurate. There is chance that DN changes to STOPPED state after return the status.

@Gargi-jais11 Gargi-jais11 marked this pull request as draft January 27, 2026 07:18
@Gargi-jais11 Gargi-jais11 reopened this Jan 28, 2026
@Gargi-jais11 Gargi-jais11 changed the title HDDS-14195. Do not send DiskBalancer start command to DN with active run HDDS-14195. [DiskBalancer] Improve DiskBalancer CLI Output Readability and Usability Jan 28, 2026
@Gargi-jais11 Gargi-jais11 changed the title HDDS-14195. [DiskBalancer] Improve DiskBalancer CLI Output Readability and Usability HDDS-14195. Do not send DiskBalancer start command to DN with active run Jan 28, 2026
@Gargi-jais11
Copy link
Contributor Author

Closing this PR, will open a new one as it is a kind of messed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants