From 15226ebe06c15623cb1957b6e57e58386a3cccd4 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 15 Dec 2025 01:12:16 -0800 Subject: [PATCH 1/5] HDDS-14175. DefaultVolumeChoosingPolicy should not call getCurrentUsage() multiple times. --- .../diskbalancer/DiskBalancerService.java | 34 ++++---- .../DiskBalancerVolumeCalculation.java | 73 ++++++++++++----- .../policy/ContainerChoosingPolicy.java | 4 +- .../DefaultContainerChoosingPolicy.java | 62 ++++---------- .../policy/DefaultVolumeChoosingPolicy.java | 81 +++++++------------ .../diskbalancer/TestDiskBalancerService.java | 11 +-- .../scm/node/TestContainerChoosingPolicy.java | 1 - .../scm/node/TestVolumeChoosingPolicy.java | 2 +- 8 files changed, 123 insertions(+), 145 deletions(-) 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..9b0379b4ec3c 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,17 +636,16 @@ private boolean tryCleanupOnePendingDeletionContainer() { } public DiskBalancerInfo getDiskBalancerInfo() { - ImmutableList immutableVolumeSet = DiskBalancerVolumeCalculation.getImmutableVolumeSet(volumeSet); + final List volumeUsages = getVolumeUsages(volumeSet); // Calculate volumeDataDensity - double volumeDatadensity = 0.0; - volumeDatadensity = DiskBalancerVolumeCalculation.calculateVolumeDataDensity(immutableVolumeSet, deltaSizes); + final double volumeDatadensity = calculateVolumeDataDensity(volumeUsages, deltaSizes); 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, @@ -650,36 +653,29 @@ public DiskBalancerInfo getDiskBalancerInfo() { 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; + final double actualThreshold = getIdealUsage(inputVolumeSet, deltaSizes) + 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.computeUtilization(deltaSizes) - 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..6fa3daa8d032 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,17 @@ package org.apache.hadoop.ozone.container.diskbalancer; +import static org.apache.ratis.util.Preconditions.assertInstanceOf; +import static org.apache.ratis.util.Preconditions.assertTrue; + import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Map; +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.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,12 +52,12 @@ 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) { + return volumeSet.getVolumesList().stream() + .map(VolumeFixedUsage::new) + .collect(Collectors.toList()); } /** @@ -66,12 +69,12 @@ public static ImmutableList getImmutableVolumeSet(MutableVolumeSet v * @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, Map deltaMap) { long totalCapacity = 0L, totalEffectiveUsed = 0L; - for (HddsVolume volume : volumes) { - SpaceUsageSource usage = volume.getCurrentUsage(); + for (VolumeFixedUsage volumeUsage : volumes) { + final HddsVolume volume = volumeUsage.getVolume(); + final SpaceUsageSource.Fixed usage = volumeUsage.getUsage(); totalCapacity += usage.getCapacity(); long currentUsed = usage.getCapacity() - usage.getAvailable(); long delta = (deltaMap != null) ? deltaMap.getOrDefault(volume, 0L) : 0L; @@ -90,7 +93,7 @@ public static double getIdealUsage(ImmutableList 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, + public static double calculateVolumeDataDensity(List volumeSet, Map deltaMap) { if (volumeSet == null) { LOG.warn("VolumeSet is null, returning 0.0 for VolumeDataDensity"); @@ -108,14 +111,9 @@ public static double calculateVolumeDataDensity(ImmutableList volume 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); + for (VolumeFixedUsage volumeUsage : volumeSet) { + final double currentUsage = volumeUsage.computeUtilization(deltaMap); - long deltaSize = (deltaMap != null) ? deltaMap.getOrDefault(volume, 0L) : 0L; - double currentUsage = (double)((usage.getCapacity() - usage.getAvailable()) - + deltaSize + volume.getCommittedBytes()) / usage.getCapacity(); - // Calculate density as absolute difference from ideal usage double volumeDensity = Math.abs(currentUsage - idealUsage); volumeDensitySum += volumeDensity; @@ -126,4 +124,41 @@ public static double calculateVolumeDataDensity(ImmutableList volume return -1.0; } } + + public static double computeUtilization(HddsVolume volume, long delta) { + return computeUtilization(volume.getCurrentUsage(), volume.getCommittedBytes(), delta); + } + + private static double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long delta) { + assertTrue(usage.getCapacity() > 0); + return (usage.getCapacity() - usage.getAvailable() + committed + delta) / (double) usage.getCapacity(); + } + + /** {@link HddsVolume} with a {@link SpaceUsageSource.Fixed} usage. */ + public static class VolumeFixedUsage { + private final HddsVolume volume; + private final SpaceUsageSource.Fixed usage; + + public VolumeFixedUsage(HddsVolume volume) { + this.volume = volume; + this.usage = volume.getCurrentUsage(); + } + + public VolumeFixedUsage(StorageVolume volume) { + this(assertInstanceOf(volume, HddsVolume.class)); + } + + public HddsVolume getVolume() { + return volume; + } + + public SpaceUsageSource.Fixed getUsage() { + return usage; + } + + public double computeUtilization(Map deltas) { + final long delta = deltas == null ? 0L : deltas.getOrDefault(volume, 0L); + return DiskBalancerVolumeCalculation.computeUtilization(usage, volume.getCommittedBytes(), delta); + } + } } 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..b0b2d56dc04e 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 * @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..ff03c6457d37 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,22 +18,24 @@ 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; -import org.apache.hadoop.hdds.fs.SpaceUsageSource; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.ozone.container.common.impl.ContainerData; 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,24 +56,23 @@ 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 actual threshold + final List volumeUsages = getVolumeUsages(volumeSet); + double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100.0; + // Find container while (itr.hasNext()) { ContainerData containerData = itr.next().getContainerData(); if (!inProgressContainerIDs.contains( @@ -79,47 +80,16 @@ public ContainerData chooseContainer(OzoneContainer ozoneContainer, (containerData.isClosed() || (test && containerData.isQuasiClosed()))) { // This is a candidate container. Now, check if moving it would be productive. - if (isMoveProductive(containerData, destVolume, maxAllowedUtilization)) { + if (computeUtilization(dst, 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..d12554757517 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 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,42 @@ 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(); - - 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 usages and sort in ascending order + final List volumeUsages = allVolumes.stream() + .map(VolumeFixedUsage::new) + .sorted(Comparator.comparingDouble(v -> v.computeUtilization(deltaMap))) + .collect(Collectors.toList()); - 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()); - - // 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; + // Find src + final double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100; + final VolumeFixedUsage src = volumeUsages.get(volumeUsages.size() - 1); + if (src.computeUtilization(deltaMap) < actualThreshold) { + return null; // all volumes are under the threshold } + + // Find dst 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; + for (int i = 0; i < volumeUsages.size() - 1; i++) { + final HddsVolume dst = volumeUsages.get(i).getVolume(); + if (filter.test(dst)) { + // 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/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..595f82bf1345 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); // 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"); } From b9e0bf4f85270b5b871666a3e896407615e68021 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 15 Dec 2025 13:19:48 -0800 Subject: [PATCH 2/5] Do not use AvailableSpaceFilter since it calls getCurrentUsage(). --- .../ozone/container/common/volume/VolumeUsage.java | 3 +-- .../container/diskbalancer/DiskBalancerService.java | 2 +- .../diskbalancer/DiskBalancerVolumeCalculation.java | 8 +++++++- .../diskbalancer/policy/ContainerChoosingPolicy.java | 2 +- .../policy/DefaultContainerChoosingPolicy.java | 4 ++-- .../policy/DefaultVolumeChoosingPolicy.java | 12 ++++++------ 6 files changed, 18 insertions(+), 13 deletions(-) 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..ed6ab2a3880b 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,8 +175,7 @@ public long getReservedInBytes() { return reservedInBytes; } - private static long getUsableSpace( - long available, long committed, long minFreeSpace) { + public static long getUsableSpace(long available, long committed, long minFreeSpace) { return available - committed - minFreeSpace; } 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 9b0379b4ec3c..9c5f27275fb3 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 @@ -659,7 +659,7 @@ public long calculateBytesToMove(List inputVolumeSet) { return 0; } - // Calculate ideal usage + // Calculate actual threshold final double actualThreshold = getIdealUsage(inputVolumeSet, deltaSizes) + threshold / 100.0; long totalBytesToMove = 0; 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 6fa3daa8d032..ea199e342e11 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 @@ -28,6 +28,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.common.volume.VolumeUsage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -131,7 +132,7 @@ public static double computeUtilization(HddsVolume volume, long delta) { private static double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long delta) { assertTrue(usage.getCapacity() > 0); - return (usage.getCapacity() - usage.getAvailable() + committed + delta) / (double) usage.getCapacity(); + return 1 - (usage.getAvailable() + committed + delta) / (double) usage.getCapacity(); } /** {@link HddsVolume} with a {@link SpaceUsageSource.Fixed} usage. */ @@ -160,5 +161,10 @@ public double computeUtilization(Map deltas) { final long delta = deltas == null ? 0L : deltas.getOrDefault(volume, 0L); return DiskBalancerVolumeCalculation.computeUtilization(usage, volume.getCommittedBytes(), delta); } + + public long computeUsableSpace() { + final long minFreeSpace = volume.getFreeSpaceToSpare(usage.getCapacity()); + return VolumeUsage.getUsableSpace(usage.getAvailable(), volume.getCommittedBytes(), minFreeSpace); + } } } 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 b0b2d56dc04e..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 thresholdPercentage the threshold percentage + * @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 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 ff03c6457d37..5dbde73047ac 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 @@ -68,9 +68,9 @@ public ContainerData chooseContainer(OzoneContainer ozoneContainer, return null; } - // Calculate actual threshold + // Calculate the actual threshold final List volumeUsages = getVolumeUsages(volumeSet); - double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100.0; + final double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100.0; // Find container while (itr.hasNext()) { 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 d12554757517..52ff376d6d7f 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 @@ -25,7 +25,6 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; -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.common.volume.StorageVolume; @@ -61,13 +60,13 @@ public Pair chooseVolume(MutableVolumeSet volumeSet, return null; // Can't balance with less than 2 volumes. } - // Calculate usages and sort in ascending order + // Calculate usages and sort in ascending order of utilization final List volumeUsages = allVolumes.stream() .map(VolumeFixedUsage::new) .sorted(Comparator.comparingDouble(v -> v.computeUtilization(deltaMap))) .collect(Collectors.toList()); - // Find src + // Calculate the actual threshold and check src final double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100; final VolumeFixedUsage src = volumeUsages.get(volumeUsages.size() - 1); if (src.computeUtilization(deltaMap) < actualThreshold) { @@ -75,10 +74,11 @@ public Pair chooseVolume(MutableVolumeSet volumeSet, } // Find dst - AvailableSpaceFilter filter = new AvailableSpaceFilter(containerSize); for (int i = 0; i < volumeUsages.size() - 1; i++) { - final HddsVolume dst = volumeUsages.get(i).getVolume(); - if (filter.test(dst)) { + final VolumeFixedUsage dstUsage = volumeUsages.get(i); + final HddsVolume dst = dstUsage.getVolume(); + + if (containerSize < dstUsage.computeUsableSpace()) { // Found dst, reserve space and return dst.incCommittedBytes(containerSize); return Pair.of(src.getVolume(), dst); From 990924a982219c2ae35ca1cb8cc36a6f9e2f118c Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Mon, 15 Dec 2025 17:25:34 -0800 Subject: [PATCH 3/5] Fix a bug --- .../diskbalancer/DiskBalancerVolumeCalculation.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 ea199e342e11..655639c21d56 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 @@ -126,13 +126,14 @@ public static double calculateVolumeDataDensity(List volumeSet } } - public static double computeUtilization(HddsVolume volume, long delta) { - return computeUtilization(volume.getCurrentUsage(), volume.getCommittedBytes(), delta); + public static double computeUtilization(HddsVolume volume, long required) { + return computeUtilization(volume.getCurrentUsage(), volume.getCommittedBytes(), required); } - private static double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long delta) { - assertTrue(usage.getCapacity() > 0); - return 1 - (usage.getAvailable() + committed + delta) / (double) usage.getCapacity(); + private static double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long required) { + final long capacity = usage.getCapacity(); + assertTrue(capacity > 0); + return (capacity - usage.getAvailable() + committed + required) / (double) capacity; } /** {@link HddsVolume} with a {@link SpaceUsageSource.Fixed} usage. */ From 905e0f4d96f25633a2e095e07f4db5f0c569b62d Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Tue, 16 Dec 2025 10:21:40 -0800 Subject: [PATCH 4/5] Address review comments and get delta only once. --- .../container/common/volume/VolumeUsage.java | 4 +- .../diskbalancer/DiskBalancerService.java | 10 +-- .../DiskBalancerVolumeCalculation.java | 70 +++++++++---------- .../DefaultContainerChoosingPolicy.java | 12 ++-- .../policy/DefaultVolumeChoosingPolicy.java | 9 +-- .../DiskBalancerVolumeChoosingPolicy.java | 4 +- .../diskbalancer/TestDiskBalancerService.java | 2 +- 7 files changed, 58 insertions(+), 53 deletions(-) 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 ed6ab2a3880b..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,8 +175,8 @@ public long getReservedInBytes() { return reservedInBytes; } - public 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 9c5f27275fb3..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 @@ -636,10 +636,10 @@ private boolean tryCleanupOnePendingDeletionContainer() { } public DiskBalancerInfo getDiskBalancerInfo() { - final List volumeUsages = getVolumeUsages(volumeSet); + final List volumeUsages = getVolumeUsages(volumeSet, deltaSizes); // Calculate volumeDataDensity - final double volumeDatadensity = calculateVolumeDataDensity(volumeUsages, deltaSizes); + final double volumeDataDensity = calculateVolumeDataDensity(volumeUsages); long bytesToMove = 0; if (this.operationalState == DiskBalancerRunningStatus.RUNNING) { @@ -650,7 +650,7 @@ public DiskBalancerInfo getDiskBalancerInfo() { 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(List inputVolumeSet) { @@ -660,7 +660,7 @@ public long calculateBytesToMove(List inputVolumeSet) { } // Calculate actual threshold - final double actualThreshold = getIdealUsage(inputVolumeSet, deltaSizes) + threshold / 100.0; + final double actualThreshold = getIdealUsage(inputVolumeSet) + threshold / 100.0; long totalBytesToMove = 0; @@ -671,7 +671,7 @@ public long calculateBytesToMove(List inputVolumeSet) { continue; } - final double excess = volumeUsage.computeUtilization(deltaSizes) - actualThreshold; + final double excess = volumeUsage.getUtilization() - actualThreshold; // Only consider volumes that exceed the threshold (source volumes) if (excess > 0) { // Calculate excess bytes that need to be moved from this volume 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 655639c21d56..7f44454b6e1c 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 @@ -20,9 +20,9 @@ import static org.apache.ratis.util.Preconditions.assertInstanceOf; import static org.apache.ratis.util.Preconditions.assertTrue; -import com.google.common.base.Preconditions; 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.volume.HddsVolume; @@ -51,13 +51,13 @@ private DiskBalancerVolumeCalculation() { /** * Get an immutable snapshot of volumes from a MutableVolumeSet. - * + * * @param volumeSet The MutableVolumeSet to create a snapshot from * @return a list of volumes and usages */ - public static List getVolumeUsages(MutableVolumeSet volumeSet) { + public static List getVolumeUsages(MutableVolumeSet volumeSet, Map deltas) { return volumeSet.getVolumesList().stream() - .map(VolumeFixedUsage::new) + .map(v -> newVolumeFixedUsage(v, deltas)) .collect(Collectors.toList()); } @@ -65,25 +65,18 @@ public static List getVolumeUsages(MutableVolumeSet volumeSet) * 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(List volumes, Map deltaMap) { + public static double getIdealUsage(List volumes) { long totalCapacity = 0L, totalEffectiveUsed = 0L; for (VolumeFixedUsage volumeUsage : volumes) { - final HddsVolume volume = volumeUsage.getVolume(); - final SpaceUsageSource.Fixed usage = volumeUsage.getUsage(); - 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); + totalCapacity += volumeUsage.getUsage().getCapacity(); + totalEffectiveUsed += volumeUsage.getEffectiveUsed(); } - Preconditions.checkArgument(totalCapacity != 0); return ((double) (totalEffectiveUsed)) / totalCapacity; } @@ -91,11 +84,9 @@ public static double getIdealUsage(List volumes, Map 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; @@ -108,12 +99,12 @@ public static double calculateVolumeDataDensity(List volumeSet } // 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 (VolumeFixedUsage volumeUsage : volumeSet) { - final double currentUsage = volumeUsage.computeUtilization(deltaMap); + final double currentUsage = volumeUsage.getUtilization(); // Calculate density as absolute difference from ideal usage double volumeDensity = Math.abs(currentUsage - idealUsage); @@ -126,28 +117,34 @@ public static double calculateVolumeDataDensity(List volumeSet } } - public static double computeUtilization(HddsVolume volume, long required) { - return computeUtilization(volume.getCurrentUsage(), volume.getCommittedBytes(), required); + 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 double computeUtilization(SpaceUsageSource.Fixed usage, long committed, long required) { - final long capacity = usage.getCapacity(); - assertTrue(capacity > 0); - return (capacity - usage.getAvailable() + 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 class VolumeFixedUsage { private final HddsVolume volume; private final SpaceUsageSource.Fixed usage; + private final long effectiveUsed; + private final Double utilization; - public VolumeFixedUsage(HddsVolume volume) { + private VolumeFixedUsage(HddsVolume volume, long delta) { this.volume = volume; this.usage = volume.getCurrentUsage(); - } - - public VolumeFixedUsage(StorageVolume volume) { - this(assertInstanceOf(volume, HddsVolume.class)); + this.effectiveUsed = computeEffectiveUsage(usage, volume.getCommittedBytes(), delta); + this.utilization = usage.getCapacity() > 0 ? computeUtilization(usage, volume.getCommittedBytes(), delta) : null; } public HddsVolume getVolume() { @@ -158,14 +155,17 @@ public SpaceUsageSource.Fixed getUsage() { return usage; } - public double computeUtilization(Map deltas) { - final long delta = deltas == null ? 0L : deltas.getOrDefault(volume, 0L); - return DiskBalancerVolumeCalculation.computeUtilization(usage, volume.getCommittedBytes(), delta); + public long getEffectiveUsed() { + return effectiveUsed; + } + + public double getUtilization() { + return Objects.requireNonNull(utilization, "utilization == null"); } public long computeUsableSpace() { - final long minFreeSpace = volume.getFreeSpaceToSpare(usage.getCapacity()); - return VolumeUsage.getUsableSpace(usage.getAvailable(), volume.getCommittedBytes(), minFreeSpace); + 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/DefaultContainerChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/policy/DefaultContainerChoosingPolicy.java index 5dbde73047ac..2a674d6c3e23 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 @@ -30,6 +30,8 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; + +import org.apache.hadoop.hdds.fs.SpaceUsageSource; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.ozone.container.common.impl.ContainerData; import org.apache.hadoop.ozone.container.common.interfaces.Container; @@ -69,18 +71,20 @@ public ContainerData chooseContainer(OzoneContainer ozoneContainer, } // Calculate the actual threshold - final List volumeUsages = getVolumeUsages(volumeSet); - final double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100.0; + 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 (computeUtilization(dst, containerData.getBytesUsed()) < actualThreshold) { + // Check if dst can accept the candidate container. + if (computeUtilization(dstUsage, dstCommittedBytes, containerData.getBytesUsed()) < actualThreshold) { return containerData; } } 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 52ff376d6d7f..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 @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.container.diskbalancer.policy; 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; @@ -62,14 +63,14 @@ public Pair chooseVolume(MutableVolumeSet volumeSet, // Calculate usages and sort in ascending order of utilization final List volumeUsages = allVolumes.stream() - .map(VolumeFixedUsage::new) - .sorted(Comparator.comparingDouble(v -> v.computeUtilization(deltaMap))) + .map(v -> newVolumeFixedUsage(v, deltaMap)) + .sorted(Comparator.comparingDouble(VolumeFixedUsage::getUtilization)) .collect(Collectors.toList()); // Calculate the actual threshold and check src - final double actualThreshold = getIdealUsage(volumeUsages, deltaMap) + thresholdPercentage / 100; + final double actualThreshold = getIdealUsage(volumeUsages) + thresholdPercentage / 100; final VolumeFixedUsage src = volumeUsages.get(volumeUsages.size() - 1); - if (src.computeUtilization(deltaMap) < actualThreshold) { + if (src.getUtilization() < actualThreshold) { return null; // all volumes are under the threshold } 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 595f82bf1345..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 @@ -253,7 +253,7 @@ public void testCalculateBytesToMove(int volumeCount, int deltaUsagePercent, long expectedBytesToMove = (long) Math.ceil( (totalCapacity * expectedBytesToMovePercent) / 100.0 * totalOverUtilisedVolumes); - final List volumeUsages = getVolumeUsages(volumeSet); + final List volumeUsages = getVolumeUsages(volumeSet, null); // data precision loss due to double data involved in calculation assertTrue(Math.abs(expectedBytesToMove - svc.calculateBytesToMove(volumeUsages)) <= 1); } From c106e8cc4a80b13ae8b0db06839fa873a8cd0d59 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Tue, 16 Dec 2025 10:30:32 -0800 Subject: [PATCH 5/5] Fix checkstyle --- .../container/diskbalancer/DiskBalancerVolumeCalculation.java | 2 +- .../diskbalancer/policy/DefaultContainerChoosingPolicy.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) 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 7f44454b6e1c..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 @@ -134,7 +134,7 @@ public static VolumeFixedUsage newVolumeFixedUsage(StorageVolume volume, Map