Gargi-jais11 commented on code in PR #11322:
URL: https://github.com/apache/ozone/pull/11322#discussion_r4205335867
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerVolumeCalculation.java:
##########
Review Comment:
`getIdealUsage` is not considering Storage Type. It should be per Storage
type evaluated.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java:
##########
@@ -717,26 +724,57 @@ private boolean tryCleanupOnePendingDeletionContainer() {
public DiskBalancerInfo getDiskBalancerInfo() {
final List<VolumeFixedUsage> volumeUsages = getVolumeUsages(volumeSet,
deltaSizes);
-
- // Calculate volumeDataDensity
- final double volumeDataDensity = calculateVolumeDataDensity(volumeUsages);
-
- long bytesToMove = 0;
- if (this.operationalState == DiskBalancerRunningStatus.RUNNING) {
- // this calculates live changes in bytesToMove
- // calculate bytes to move if the balancer is in a running state, else 0.
- bytesToMove = calculateBytesToMove(volumeUsages);
- }
+ final List<StorageTypeDiskBalancerInfoProto> storageTypeInfo =
+ buildStorageTypeInfo(volumeUsages, threshold,
+ this.operationalState == DiskBalancerRunningStatus.RUNNING);
+ final double volumeDataDensity = storageTypeInfo.stream()
+
.mapToDouble(StorageTypeDiskBalancerInfoProto::getCurrentVolumeDensitySum)
+ .sum();
+ final long bytesToMove = storageTypeInfo.stream()
+ .mapToLong(StorageTypeDiskBalancerInfoProto::getBytesToMove)
+ .sum();
DiskBalancerInfo info = new DiskBalancerInfo(operationalState, threshold,
bandwidthInMB,
parallelThread, stopAfterDiskEven, version,
formatContainerStates(movableContainerStates),
metrics.getSuccessCount(),
metrics.getFailureCount(), bytesToMove, metrics.getSuccessBytes(),
volumeDataDensity);
info.setIdealUsage(getIdealUsage(volumeUsages));
info.setVolumeInfo(buildVolumeReportProto(volumeUsages));
+ info.setStorageTypeInfo(storageTypeInfo);
return info;
}
+ @VisibleForTesting
Review Comment:
`@VisibleFprTesting` is no more supported. Refernce:
https://issues.apache.org/jira/browse/HDDS-12725
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java:
##########
@@ -91,62 +99,123 @@ 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. Try the type
+ // with the largest threshold violation first. StorageType order is only
a deterministic
+ // tie-breaker.
+ final Map<StorageType, List<VolumeFixedUsage>> usagesByStorageType =
+ getUsableVolumesByStorageType(volumeUsages);
+ final List<Map.Entry<StorageType, List<VolumeFixedUsage>>>
storageTypeGroups =
+ usagesByStorageType.entrySet().stream()
+ .filter(entry -> hasMultipleUsableVolumes(entry.getKey(),
entry.getValue()))
+ .sorted(Comparator
+ .<Map.Entry<StorageType,
List<VolumeFixedUsage>>>comparingDouble(
+ entry -> getThresholdViolation(entry.getValue(),
thresholdPercentage))
+ .reversed()
+ .thenComparingInt(entry -> entry.getKey().ordinal()))
+ .collect(Collectors.toList());
+ for (Map.Entry<StorageType, List<VolumeFixedUsage>> entry :
storageTypeGroups) {
+ final StorageType storageType = entry.getKey();
+ final List<VolumeFixedUsage> sameTypeUsages = entry.getValue();
+ final ContainerCandidate candidate =
chooseWithinStorageType(ozoneContainer, sameTypeUsages,
+ deltaMap, inProgressContainerIDs, thresholdPercentage,
movableContainerStates, storageType);
+ if (candidate != null) {
+ return candidate;
+ }
}
+ LOG.debug("Failed to find appropriate destination volume and container
in any storage type.");
+ return null;
+ } finally {
+ lock.unlock();
+ }
+ }
- // Get highest and lowest utilization volumes
- final VolumeFixedUsage highestUsage =
volumeUsages.get(volumeUsages.size() - 1);
- final VolumeFixedUsage lowestUsage = volumeUsages.get(0);
+ private static boolean hasMultipleUsableVolumes(StorageType storageType,
+ List<VolumeFixedUsage> volumeUsages) {
+ if (volumeUsages.size() >= 2) {
+ return true;
+ }
+ LOG.debug("Skipping storage type {} for disk balancing: only {} usable
volume(s)",
+ storageType, volumeUsages.size());
+ return false;
+ }
- // Only return null if highest is below upper threshold AND lowest is
above lower threshold
- if (highestUsage.getUtilization() < upperThreshold &&
- lowestUsage.getUtilization() > lowerThreshold) {
- return null;
- }
+ private static double getThresholdViolation(List<VolumeFixedUsage>
volumeUsages,
Review Comment:
Many new helper methods are added and calling `VolumeUsages`. These helper
methods should be moved to `DiskBalancerVolumeCalaculation` class as the logic
can be reused and Each calculation will get one time snapshot of the
VolumeUsages.
##########
hadoop-hdds/interface-client/src/main/proto/hdds.proto:
##########
@@ -608,6 +608,16 @@ message VolumeReportProto {
optional uint64 effectiveUsedSpace = 6;
optional double utilization = 7;
optional uint64 ozoneAvailable = 8;
+ optional StorageTypeProto storageType = 9;
+}
+
+message StorageTypeDiskBalancerInfoProto {
Review Comment:
Please check the existing proto messages it those can be reused else we can
keep the new ones.
##########
hadoop-hdds/interface-client/src/main/resources/proto.lock:
##########
@@ -4625,6 +4625,53 @@
"name": "ozoneAvailable",
"type": "uint64",
"optional": true
+ },
Review Comment:
Do we really need in this PR?
##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerReportSubcommand.java:
##########
@@ -123,7 +125,10 @@ private String generateReport(
.append(formatPercent(p.getCurrentVolumeDensitySum()))
.append(System.lineSeparator());
- if (p.hasIdealUsage() && p.hasDiskBalancerConf()
+ if (p.getStorageTypeInfoCount() > 0 && p.hasDiskBalancerConf()
+ && p.getDiskBalancerConf().hasThreshold()) {
+ appendStorageTypeDetails(header, p);
+ } else if (p.hasIdealUsage() && p.hasDiskBalancerConf()
&& p.getDiskBalancerConf().hasThreshold()) {
double idealUsage = p.getIdealUsage();
Review Comment:
here in report we should display idealUsage for each storageType so that
it's easy for user to compare.
--
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]
