diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java index 907ec6112456..15275fcd6c59 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java @@ -175,9 +175,8 @@ public long getReservedInBytes() { return reservedInBytes; } - private static long getUsableSpace( - long available, long committed, long minFreeSpace) { - return available - committed - minFreeSpace; + public static long getUsableSpace(long available, long committed, long spared) { + return available - committed - spared; } public static long getUsableSpace(StorageReportProto report) { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java index 37984848e9ff..7490c5bb3eaa 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java @@ -18,16 +18,19 @@ package org.apache.hadoop.ozone.container.diskbalancer; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_ALREADY_EXISTS; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.calculateVolumeDataDensity; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getIdealUsage; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getVolumeUsages; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardCopyOption; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -65,6 +68,7 @@ import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; import org.apache.hadoop.ozone.container.common.volume.VolumeChoosingPolicyFactory; +import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.VolumeFixedUsage; import org.apache.hadoop.ozone.container.diskbalancer.policy.ContainerChoosingPolicy; import org.apache.hadoop.ozone.container.diskbalancer.policy.DiskBalancerVolumeChoosingPolicy; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; @@ -632,54 +636,46 @@ private boolean tryCleanupOnePendingDeletionContainer() { } public DiskBalancerInfo getDiskBalancerInfo() { - ImmutableList immutableVolumeSet = DiskBalancerVolumeCalculation.getImmutableVolumeSet(volumeSet); + final List volumeUsages = getVolumeUsages(volumeSet, deltaSizes); // Calculate volumeDataDensity - double volumeDatadensity = 0.0; - volumeDatadensity = DiskBalancerVolumeCalculation.calculateVolumeDataDensity(immutableVolumeSet, deltaSizes); + 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(immutableVolumeSet); + bytesToMove = calculateBytesToMove(volumeUsages); } return new DiskBalancerInfo(operationalState, threshold, bandwidthInMB, parallelThread, stopAfterDiskEven, version, metrics.getSuccessCount(), - metrics.getFailureCount(), bytesToMove, metrics.getSuccessBytes(), volumeDatadensity); + metrics.getFailureCount(), bytesToMove, metrics.getSuccessBytes(), volumeDataDensity); } - public long calculateBytesToMove(ImmutableList inputVolumeSet) { + public long calculateBytesToMove(List inputVolumeSet) { // If there are no available volumes or only one volume, return 0 bytes to move if (inputVolumeSet.isEmpty() || inputVolumeSet.size() < 2) { return 0; } - // Calculate ideal usage - double idealUsage = DiskBalancerVolumeCalculation.getIdealUsage(inputVolumeSet, deltaSizes); - double normalizedThreshold = threshold / 100.0; + // Calculate actual threshold + final double actualThreshold = getIdealUsage(inputVolumeSet) + threshold / 100.0; long totalBytesToMove = 0; // Calculate excess data in overused volumes - for (HddsVolume volume : inputVolumeSet) { - SpaceUsageSource usage = volume.getCurrentUsage(); - + for (VolumeFixedUsage volumeUsage : inputVolumeSet) { + final SpaceUsageSource.Fixed usage = volumeUsage.getUsage(); if (usage.getCapacity() == 0) { continue; } - long deltaSize = deltaSizes.getOrDefault(volume, 0L); - double currentUsage = (double)((usage.getCapacity() - usage.getAvailable()) - + deltaSize + volume.getCommittedBytes()) / usage.getCapacity(); - - double volumeUtilisation = currentUsage - idealUsage; - + final double excess = volumeUsage.getUtilization() - actualThreshold; // Only consider volumes that exceed the threshold (source volumes) - if (volumeUtilisation >= normalizedThreshold) { + if (excess > 0) { // Calculate excess bytes that need to be moved from this volume - long excessBytes = (long) ((volumeUtilisation - normalizedThreshold) * usage.getCapacity()); + final long excessBytes = (long) (excess * usage.getCapacity()); totalBytesToMove += Math.max(0, excessBytes); } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerVolumeCalculation.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerVolumeCalculation.java index 442cdcface41..45ac5c4658f0 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerVolumeCalculation.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerVolumeCalculation.java @@ -17,14 +17,18 @@ package org.apache.hadoop.ozone.container.diskbalancer; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; +import static org.apache.ratis.util.Preconditions.assertInstanceOf; +import static org.apache.ratis.util.Preconditions.assertTrue; + import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; import org.apache.hadoop.hdds.fs.SpaceUsageSource; -import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil; import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; +import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.apache.hadoop.ozone.container.common.volume.VolumeUsage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,39 +51,32 @@ private DiskBalancerVolumeCalculation() { /** * Get an immutable snapshot of volumes from a MutableVolumeSet. - * + * * @param volumeSet The MutableVolumeSet to create a snapshot from - * @return Immutable list of HddsVolume objects + * @return a list of volumes and usages */ - public static ImmutableList getImmutableVolumeSet(MutableVolumeSet volumeSet) { - // Create an immutable copy of the volume list at this point in time - List volumes = StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()); - return ImmutableList.copyOf(volumes); + public static List getVolumeUsages(MutableVolumeSet volumeSet, Map deltas) { + return volumeSet.getVolumesList().stream() + .map(v -> newVolumeFixedUsage(v, deltas)) + .collect(Collectors.toList()); } /** * Get ideal usage from an immutable list of volumes. * * @param volumes Immutable list of volumes - * @param deltaMap A map that tracks the total bytes which will be freed * from each source volume during container moves * @return Ideal usage as a ratio (used space / total capacity) * @throws IllegalArgumentException if total capacity is zero */ - public static double getIdealUsage(ImmutableList volumes, - Map deltaMap) { + public static double getIdealUsage(List volumes) { long totalCapacity = 0L, totalEffectiveUsed = 0L; - for (HddsVolume volume : volumes) { - SpaceUsageSource usage = volume.getCurrentUsage(); - totalCapacity += usage.getCapacity(); - long currentUsed = usage.getCapacity() - usage.getAvailable(); - long delta = (deltaMap != null) ? deltaMap.getOrDefault(volume, 0L) : 0L; - long committed = volume.getCommittedBytes(); - totalEffectiveUsed += (currentUsed + delta + committed); + for (VolumeFixedUsage volumeUsage : volumes) { + totalCapacity += volumeUsage.getUsage().getCapacity(); + totalEffectiveUsed += volumeUsage.getEffectiveUsed(); } - Preconditions.checkArgument(totalCapacity != 0); return ((double) (totalEffectiveUsed)) / totalCapacity; } @@ -87,11 +84,9 @@ public static double getIdealUsage(ImmutableList volumes, * Calculate VolumeDataDensity. * * @param volumeSet The MutableVolumeSet containing all volumes - * @param deltaMap Map of volume to delta sizes (ongoing operations), can be null * @return VolumeDataDensity sum across all volumes */ - public static double calculateVolumeDataDensity(ImmutableList volumeSet, - Map deltaMap) { + public static double calculateVolumeDataDensity(List volumeSet) { if (volumeSet == null) { LOG.warn("VolumeSet is null, returning 0.0 for VolumeDataDensity"); return 0.0; @@ -104,18 +99,13 @@ public static double calculateVolumeDataDensity(ImmutableList volume } // Calculate ideal usage using the same immutable volume snapshot - double idealUsage = getIdealUsage(volumeSet, deltaMap); + final double idealUsage = getIdealUsage(volumeSet); double volumeDensitySum = 0.0; // Calculate density for each volume using the same snapshot - for (HddsVolume volume : volumeSet) { - SpaceUsageSource usage = volume.getCurrentUsage(); - Preconditions.checkArgument(usage.getCapacity() != 0); - - long deltaSize = (deltaMap != null) ? deltaMap.getOrDefault(volume, 0L) : 0L; - double currentUsage = (double)((usage.getCapacity() - usage.getAvailable()) - + deltaSize + volume.getCommittedBytes()) / usage.getCapacity(); - + for (VolumeFixedUsage volumeUsage : volumeSet) { + final double currentUsage = volumeUsage.getUtilization(); + // Calculate density as absolute difference from ideal usage double volumeDensity = Math.abs(currentUsage - idealUsage); volumeDensitySum += volumeDensity; @@ -126,4 +116,56 @@ public static double calculateVolumeDataDensity(ImmutableList volume return -1.0; } } + + public static double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long required) { + final long capacity = usage.getCapacity(); + assertTrue(capacity > 0, () -> "capacity = " + capacity + " <= 0"); + return computeEffectiveUsage(usage, committed, required) / (double) capacity; + } + + private static long computeEffectiveUsage(SpaceUsageSource.Fixed usage, long committed, long required) { + return usage.getCapacity() - usage.getAvailable() + committed + required; + } + + public static VolumeFixedUsage newVolumeFixedUsage(StorageVolume volume, Map deltaMap) { + final HddsVolume v = assertInstanceOf(volume, HddsVolume.class); + final long delta = deltaMap == null ? 0 : deltaMap.getOrDefault(v, 0L); + return new VolumeFixedUsage(v, delta); + } + + /** {@link HddsVolume} with a {@link SpaceUsageSource.Fixed} usage. */ + public static final class VolumeFixedUsage { + private final HddsVolume volume; + private final SpaceUsageSource.Fixed usage; + private final long effectiveUsed; + private final Double utilization; + + private VolumeFixedUsage(HddsVolume volume, long delta) { + this.volume = volume; + this.usage = volume.getCurrentUsage(); + this.effectiveUsed = computeEffectiveUsage(usage, volume.getCommittedBytes(), delta); + this.utilization = usage.getCapacity() > 0 ? computeUtilization(usage, volume.getCommittedBytes(), delta) : null; + } + + public HddsVolume getVolume() { + return volume; + } + + public SpaceUsageSource.Fixed getUsage() { + return usage; + } + + public long getEffectiveUsed() { + return effectiveUsed; + } + + public double getUtilization() { + return Objects.requireNonNull(utilization, "utilization == null"); + } + + public long computeUsableSpace() { + final long spared = volume.getFreeSpaceToSpare(usage.getCapacity()); + return VolumeUsage.getUsableSpace(usage.getAvailable(), volume.getCommittedBytes(), spared); + } + } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/ContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/ContainerChoosingPolicy.java index da52f4fcd53f..c6bcc1ff50fd 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/ContainerChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/ContainerChoosingPolicy.java @@ -36,7 +36,7 @@ public interface ContainerChoosingPolicy { * @param destVolume the destination volume to which container is being moved. * @param inProgressContainerIDs containerIDs present in this set should be - avoided as these containers are already under move by diskBalancer. - * @param threshold the threshold value + * @param thresholdPercentage the threshold percentage in range [0, 100] * @param volumeSet the volumeSet instance * @param deltaMap the deltaMap instance of source volume * @return a Container @@ -44,6 +44,6 @@ public interface ContainerChoosingPolicy { ContainerData chooseContainer(OzoneContainer ozoneContainer, HddsVolume srcVolume, HddsVolume destVolume, Set inProgressContainerIDs, - Double threshold, MutableVolumeSet volumeSet, + double thresholdPercentage, MutableVolumeSet volumeSet, Map deltaMap); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java index c759271f881d..11728beb9437 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java @@ -18,12 +18,15 @@ package org.apache.hadoop.ozone.container.diskbalancer.policy; import static java.util.concurrent.TimeUnit.HOURS; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.computeUtilization; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getIdealUsage; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getVolumeUsages; import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.collect.ImmutableList; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -33,7 +36,7 @@ import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; -import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation; +import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.VolumeFixedUsage; import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,72 +57,42 @@ public class DefaultContainerChoosingPolicy implements ContainerChoosingPolicy { @Override public ContainerData chooseContainer(OzoneContainer ozoneContainer, - HddsVolume srcVolume, HddsVolume destVolume, + HddsVolume src, HddsVolume dst, Set inProgressContainerIDs, - Double threshold, MutableVolumeSet volumeSet, + double thresholdPercentage, MutableVolumeSet volumeSet, Map deltaMap) { - Iterator> itr; + final Iterator> itr; try { - itr = CACHE.get().get(srcVolume, - () -> ozoneContainer.getController().getContainers(srcVolume)); + itr = CACHE.get().get(src, () -> ozoneContainer.getController().getContainers(src)); } catch (ExecutionException e) { - LOG.warn("Failed to get container iterator for volume {}", srcVolume, e); + LOG.warn("Failed to get container iterator for volume {}", src, e); return null; } - // Calculate maxAllowedUtilization - ImmutableList immutableVolumeSet = DiskBalancerVolumeCalculation.getImmutableVolumeSet(volumeSet); - double idealUsage = DiskBalancerVolumeCalculation.getIdealUsage(immutableVolumeSet, deltaMap); - double maxAllowedUtilization = idealUsage + (threshold / 100.0); + // Calculate the actual threshold + final List volumeUsages = getVolumeUsages(volumeSet, deltaMap); + final double actualThreshold = getIdealUsage(volumeUsages) + thresholdPercentage / 100.0; + // Find container + final SpaceUsageSource.Fixed dstUsage = dst.getCurrentUsage(); + final long dstCommittedBytes = dst.getCommittedBytes(); while (itr.hasNext()) { ContainerData containerData = itr.next().getContainerData(); if (!inProgressContainerIDs.contains( ContainerID.valueOf(containerData.getContainerID())) && (containerData.isClosed() || (test && containerData.isQuasiClosed()))) { - // This is a candidate container. Now, check if moving it would be productive. - if (isMoveProductive(containerData, destVolume, maxAllowedUtilization)) { + // Check if dst can accept the candidate container. + if (computeUtilization(dstUsage, dstCommittedBytes, containerData.getBytesUsed()) < actualThreshold) { return containerData; } } } - if (!itr.hasNext()) { - CACHE.get().invalidate(srcVolume); - } + CACHE.get().invalidate(src); return null; } - /** - * Checks if moving the given container from source to destination would - * result in the destination's utilization being less than or equal to the - * averageUtilization + threshold. This prevents "thrashing" where a move - * immediately makes the destination a candidate for a source volume. - * - * @param containerData The container to be moved. - * @param destVolume The destination volume. - * @param maxAllowedUtilization The maximum allowed utilization - * for the destination volume. - * @return true if the move is productive, false otherwise. - */ - private boolean isMoveProductive(ContainerData containerData, HddsVolume destVolume, - Double maxAllowedUtilization) { - long containerSize = containerData.getBytesUsed(); - SpaceUsageSource usage = destVolume.getCurrentUsage(); - - double newDestUtilization = - (double) ((usage.getCapacity() - usage.getAvailable()) + destVolume.getCommittedBytes() + containerSize) - / usage.getCapacity(); - - if (newDestUtilization <= maxAllowedUtilization) { - // The move is productive. - return true; - } - - return false; - } - @VisibleForTesting public static void setTest(boolean isTest) { test = isTest; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultVolumeChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultVolumeChoosingPolicy.java index 1dee4b57d43f..c994af442cd5 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultVolumeChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultVolumeChoosingPolicy.java @@ -17,17 +17,19 @@ package org.apache.hadoop.ozone.container.diskbalancer.policy; -import com.google.common.collect.ImmutableList; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getIdealUsage; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.newVolumeFixedUsage; + +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; -import org.apache.hadoop.hdds.fs.SpaceUsageSource; -import org.apache.hadoop.ozone.container.common.volume.AvailableSpaceFilter; import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; -import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation; +import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.VolumeFixedUsage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,67 +52,43 @@ public DefaultVolumeChoosingPolicy(ReentrantLock globalLock) { @Override public Pair chooseVolume(MutableVolumeSet volumeSet, - double threshold, Map deltaMap, long containerSize) { + double thresholdPercentage, Map deltaMap, long containerSize) { lock.lock(); try { // Create truly immutable snapshot of volumes to ensure consistency - ImmutableList allVolumes = DiskBalancerVolumeCalculation.getImmutableVolumeSet(volumeSet); - + final List allVolumes = volumeSet.getVolumesList(); if (allVolumes.size() < 2) { return null; // Can't balance with less than 2 volumes. } - - // Calculate ideal usage using the same immutable volume - double idealUsage = DiskBalancerVolumeCalculation.getIdealUsage(allVolumes, deltaMap); - // Threshold is given as a percentage - double normalizedThreshold = threshold / 100; - List volumes = allVolumes - .stream() - .filter(volume -> { - SpaceUsageSource usage = volume.getCurrentUsage(); + // Calculate usages and sort in ascending order of utilization + final List volumeUsages = allVolumes.stream() + .map(v -> newVolumeFixedUsage(v, deltaMap)) + .sorted(Comparator.comparingDouble(VolumeFixedUsage::getUtilization)) + .collect(Collectors.toList()); - return Math.abs( - ((double)((usage.getCapacity() - usage.getAvailable()) - + deltaMap.getOrDefault(volume, 0L) + volume.getCommittedBytes())) - / usage.getCapacity() - idealUsage) >= normalizedThreshold; - - }).sorted((v1, v2) -> { - SpaceUsageSource usage1 = v1.getCurrentUsage(); - SpaceUsageSource usage2 = v2.getCurrentUsage(); + // Calculate the actual threshold and check src + final double actualThreshold = getIdealUsage(volumeUsages) + thresholdPercentage / 100; + final VolumeFixedUsage src = volumeUsages.get(volumeUsages.size() - 1); + if (src.getUtilization() < actualThreshold) { + return null; // all volumes are under the threshold + } - return Double.compare( - (double) ((usage2.getCapacity() - usage2.getAvailable()) - + deltaMap.getOrDefault(v2, 0L) + v2.getCommittedBytes()) / - usage2.getCapacity(), - (double) ((usage1.getCapacity() - usage1.getAvailable()) - + deltaMap.getOrDefault(v1, 0L) + v1.getCommittedBytes()) / - usage1.getCapacity()); - }).collect(Collectors.toList()); + // Find dst + for (int i = 0; i < volumeUsages.size() - 1; i++) { + final VolumeFixedUsage dstUsage = volumeUsages.get(i); + final HddsVolume dst = dstUsage.getVolume(); - // Can not generate DiskBalancerTask if we have less than 2 results - if (volumes.size() <= 1) { - LOG.debug("Can not find appropriate Source volume and Dest Volume."); - return null; - } - AvailableSpaceFilter filter = new AvailableSpaceFilter(containerSize); - HddsVolume srcVolume = volumes.get(0); - HddsVolume destVolume = volumes.get(volumes.size() - 1); - while (!filter.test(destVolume)) { - // If the destination volume does not have enough space, try the next - // one in the list. - LOG.debug("Destination volume {} does not have enough space, trying next volume.", - destVolume.getStorageID()); - volumes.remove(destVolume); - if (volumes.size() <= 1) { - LOG.debug("Can not find appropriate Source volume and Dest Volume."); - return null; + if (containerSize < dstUsage.computeUsableSpace()) { + // Found dst, reserve space and return + dst.incCommittedBytes(containerSize); + return Pair.of(src.getVolume(), dst); } - destVolume = volumes.get(volumes.size() - 1); + LOG.debug("Destination volume {} does not have enough space, trying next volume.", + dst.getStorageID()); } - // reserve space for the dest volume - destVolume.incCommittedBytes(containerSize); - return Pair.of(srcVolume, destVolume); + LOG.debug("Failed to find appropriate destination volume."); + return null; } finally { lock.unlock(); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DiskBalancerVolumeChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DiskBalancerVolumeChoosingPolicy.java index 01520fea37b3..043aa83f5f9f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DiskBalancerVolumeChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DiskBalancerVolumeChoosingPolicy.java @@ -30,11 +30,11 @@ public interface DiskBalancerVolumeChoosingPolicy { * Choose a pair of volumes for balancing. * * @param volumeSet - volumes to choose from. - * @param threshold - the threshold to choose source and dest volumes. + * @param thresholdPercentage the threshold percentage in range [0, 100] to choose the source volume. * @param deltaSizes - the sizes changes of inProgress balancing jobs. * @param containerSize - the estimated size of container to be moved. * @return Source volume and Dest volume. */ Pair chooseVolume(MutableVolumeSet volumeSet, - double threshold, Map deltaSizes, long containerSize); + double thresholdPercentage, Map deltaSizes, long containerSize); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerService.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerService.java index 5bd84c2086fc..ca5dc7108563 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerService.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerService.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.container.diskbalancer; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded; +import static org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.getVolumeUsages; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -27,7 +28,6 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import com.google.common.collect.ImmutableList; import java.io.File; import java.io.IOException; import java.nio.file.Path; @@ -51,6 +51,7 @@ import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.apache.hadoop.ozone.container.diskbalancer.DiskBalancerVolumeCalculation.VolumeFixedUsage; import org.apache.hadoop.ozone.container.diskbalancer.policy.ContainerChoosingPolicy; import org.apache.hadoop.ozone.container.diskbalancer.policy.DefaultContainerChoosingPolicy; import org.apache.hadoop.ozone.container.diskbalancer.policy.DefaultVolumeChoosingPolicy; @@ -252,10 +253,9 @@ public void testCalculateBytesToMove(int volumeCount, int deltaUsagePercent, long expectedBytesToMove = (long) Math.ceil( (totalCapacity * expectedBytesToMovePercent) / 100.0 * totalOverUtilisedVolumes); - ImmutableList immutableVolumes = DiskBalancerVolumeCalculation.getImmutableVolumeSet(volumeSet); - + final List volumeUsages = getVolumeUsages(volumeSet, null); // data precision loss due to double data involved in calculation - assertTrue(Math.abs(expectedBytesToMove - svc.calculateBytesToMove(immutableVolumes)) <= 1); + assertTrue(Math.abs(expectedBytesToMove - svc.calculateBytesToMove(volumeUsages)) <= 1); } @Test @@ -293,7 +293,8 @@ public void testConcurrentTasksNotExceedThreadLimit() throws Exception { when(containerData.getBytesUsed()).thenReturn(100L); when(volumePolicy.chooseVolume(any(), anyDouble(), any(), anyLong())).thenReturn(Pair.of(source, dest)); - when(containerPolicy.chooseContainer(any(), any(), any(), any(), any(), any(), any())).thenReturn(containerData); + when(containerPolicy.chooseContainer(any(), any(), any(), any(), anyDouble(), any(), any())) + .thenReturn(containerData); // Test when no tasks are in progress, it should schedule up to the limit BackgroundTaskQueue queue = svc.getTasks(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java index efebf0352538..15c39626564c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestContainerChoosingPolicy.java @@ -279,7 +279,6 @@ public void createContainers() { containerSet.addContainer(container); // Add container to ContainerSet } catch (Exception e) { Assertions.fail(e.getMessage()); - throw new RuntimeException("Failed to add container to ContainerSet", e); } // Collect IDs of closed containers diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java index b65cfa637c83..1bb20c1ce716 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestVolumeChoosingPolicy.java @@ -262,7 +262,7 @@ private void createVolumes() throws IOException { } // Initialize the volumeSet with the new volume map - volumeSet.setVolumeMap(newVolumeMap); + volumeSet.setVolumeMapForTesting(newVolumeMap); System.out.println("Created " + NUM_VOLUMES + " volumes in " + (System.currentTimeMillis() - startTime) + " ms"); }