devmadhuu commented on code in PR #11322:
URL: https://github.com/apache/ozone/pull/11322#discussion_r4197790903
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java:
##########
Review Comment:
Agreed. After this patch, the balancing area is each `StorageType`
independently, but `getDiskBalancerInfo()` for CLI still calculates its ideal
usage, density, and estimated bytes across the entire volume set.
I have aligned reporting with the choosing policy by grouping usable volumes
by `StorageType` and calculating ideal usage, density, and bytes to move
independently for each balanceable group. Types with fewer than two usable
volumes will contribute zero actionable work but will remain visible in the
volume report.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java:
##########
@@ -91,62 +98,98 @@ public ContainerCandidate
chooseVolumesAndContainer(OzoneContainer ozoneContaine
return null;
}
- // Calculate ideal usage and threshold range (once)
- final double idealUsage = getIdealUsage(volumeUsages);
- final double actualThreshold = thresholdPercentage / 100.0;
- final double lowerThreshold = idealUsage - actualThreshold;
- final double upperThreshold = idealUsage + actualThreshold;
-
- if (LOG.isDebugEnabled()) {
- logVolumeBalancingState(volumeUsages, idealUsage, thresholdPercentage,
- lowerThreshold, upperThreshold, deltaMap);
+ // Balance each storage type independently so a container never crosses
types. Iterate
+ // StorageType.values() rather than the grouping keys to keep the order
stable across runs.
+ final Map<StorageType, List<VolumeFixedUsage>> usagesByStorageType =
volumeUsages.stream()
+ .collect(Collectors.groupingBy(v -> v.getVolume().getStorageType()));
+ for (StorageType storageType : StorageType.values()) {
Review Comment:
Thanks, this is a valid concern. `getTasks()` invokes the policy once per
available worker, and the policy currently restarts `StorageType.values()` on
every invocation. Therefore, the first eligible type can consume every worker
slot. I propose ranking eligible storage-type groups by their current threshold
violation. We would then try groups in descending severity order, falling
through when a group has no movable container. The source delta and destination
committed-byte accounting are already updated between policy calls, so the
ranking naturally changes as moves are scheduled. I prefer deterministic
severity ranking over unconditional rotation because rotation can prioritize a
mildly imbalanced tier over a volume close to full. `StorageType` order can
remain only as a deterministic tie-breaker. I have also updated the test so
DISK has the larger imbalance while SSD still appears first in the list; that
will verify that selection is based on imbalance rather than list 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: [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]
