From f48eebc18feab0893c6936894fa0282f02f2a196 Mon Sep 17 00:00:00 2001 From: slfan1989 Date: Thu, 14 May 2026 17:37:40 +0800 Subject: [PATCH 1/2] HDDS-15275. DiskBalancer getIdealUsage handles zero capacity incorrectly. --- .../DiskBalancerVolumeCalculation.java | 13 +++- .../TestDiskBalancerVolumeCalculation.java | 77 +++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java 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 45ac5c4658f0..afcdc38e86b3 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 @@ -70,13 +70,22 @@ public static List getVolumeUsages(MutableVolumeSet volumeSet, * @throws IllegalArgumentException if total capacity is zero */ public static double getIdealUsage(List volumes) { + if (volumes == null || volumes.isEmpty()) { + return 0.0; + } + long totalCapacity = 0L, totalEffectiveUsed = 0L; - + for (VolumeFixedUsage volumeUsage : volumes) { totalCapacity += volumeUsage.getUsage().getCapacity(); totalEffectiveUsed += volumeUsage.getEffectiveUsed(); } - + + if (totalCapacity <= 0) { + throw new IllegalArgumentException( + "total capacity must be greater than 0"); + } + return ((double) (totalEffectiveUsed)) / totalCapacity; } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java new file mode 100644 index 000000000000..7a301c9d29a8 --- /dev/null +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.container.diskbalancer; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.nio.file.Path; +import java.time.Duration; +import java.util.Collections; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.fs.MockSpaceUsageCheckFactory; +import org.apache.hadoop.hdds.fs.MockSpaceUsageSource; +import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory; +import org.apache.hadoop.hdds.fs.SpaceUsagePersistence; +import org.apache.hadoop.hdds.fs.SpaceUsageSource; +import org.apache.hadoop.ozone.container.common.volume.HddsVolume; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests for disk balancer volume calculations. + */ +class TestDiskBalancerVolumeCalculation { + + @TempDir + private Path tempDir; + + @Test + void getIdealUsageReturnsZeroForEmptyVolumeList() { + assertEquals(0.0, DiskBalancerVolumeCalculation.getIdealUsage( + Collections.emptyList())); + } + + @Test + void getIdealUsageRejectsZeroTotalCapacity() throws IOException { + HddsVolume zeroCapacityVolume = createVolume("zero-capacity", 0, 0); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> DiskBalancerVolumeCalculation.getIdealUsage( + Collections.singletonList( + DiskBalancerVolumeCalculation.newVolumeFixedUsage( + zeroCapacityVolume, null)))); + + assertEquals("total capacity must be greater than 0", exception.getMessage()); + } + + private HddsVolume createVolume(String name, long capacity, long available) + throws IOException { + OzoneConfiguration conf = new OzoneConfiguration(); + SpaceUsageSource source = MockSpaceUsageSource.fixed(capacity, available); + SpaceUsageCheckFactory factory = MockSpaceUsageCheckFactory.of( + source, Duration.ZERO, SpaceUsagePersistence.None.INSTANCE); + + return new HddsVolume.Builder(tempDir.resolve(name).toString()) + .conf(conf) + .usageCheckFactory(factory) + .build(); + } +} From 32af12670041c2791e6fabb856c5cdc5fef33d63 Mon Sep 17 00:00:00 2001 From: slfan1989 Date: Fri, 15 May 2026 10:50:57 +0800 Subject: [PATCH 2/2] HDDS-15275 - DiskBalancer getIdealUsage handles zero capacity incorrectly. --- .../DiskBalancerVolumeCalculation.java | 28 +++++++--- .../TestDiskBalancerVolumeCalculation.java | 52 +++++++++++++++++-- 2 files changed, 71 insertions(+), 9 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 afcdc38e86b3..89b00f073e7c 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 @@ -67,7 +67,6 @@ public static List getVolumeUsages(MutableVolumeSet volumeSet, * @param volumes Immutable list of volumes * 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) { if (volumes == null || volumes.isEmpty()) { @@ -77,13 +76,30 @@ public static double getIdealUsage(List volumes) { long totalCapacity = 0L, totalEffectiveUsed = 0L; for (VolumeFixedUsage volumeUsage : volumes) { - totalCapacity += volumeUsage.getUsage().getCapacity(); - totalEffectiveUsed += volumeUsage.getEffectiveUsed(); + final long capacity = volumeUsage.getUsage().getCapacity(); + if (capacity < 0) { + throw new IllegalArgumentException( + "Negative capacity = " + capacity + ": " + volumeUsage.getVolume()); + } + + final long effectiveUsed = volumeUsage.getEffectiveUsed(); + if (effectiveUsed < 0) { + throw new IllegalArgumentException( + "Negative effective used = " + effectiveUsed + ": " + + volumeUsage.getVolume()); + } + if (effectiveUsed > capacity) { + throw new IllegalArgumentException( + "Effective used = " + effectiveUsed + " > capacity = " + + capacity + ": " + volumeUsage.getVolume()); + } + + totalCapacity += capacity; + totalEffectiveUsed += effectiveUsed; } - if (totalCapacity <= 0) { - throw new IllegalArgumentException( - "total capacity must be greater than 0"); + if (totalCapacity == 0) { + return 0.0; } return ((double) (totalEffectiveUsed)) / totalCapacity; diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java index 7a301c9d29a8..9ea13503b056 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/diskbalancer/TestDiskBalancerVolumeCalculation.java @@ -49,17 +49,63 @@ void getIdealUsageReturnsZeroForEmptyVolumeList() { } @Test - void getIdealUsageRejectsZeroTotalCapacity() throws IOException { + void getIdealUsageReturnsZeroForZeroTotalCapacity() throws IOException { HddsVolume zeroCapacityVolume = createVolume("zero-capacity", 0, 0); + assertEquals(0.0, DiskBalancerVolumeCalculation.getIdealUsage( + Collections.singletonList( + DiskBalancerVolumeCalculation.newVolumeFixedUsage( + zeroCapacityVolume, null)))); + } + + @Test + void getIdealUsageRejectsNegativeCapacity() throws IOException { + HddsVolume negativeCapacityVolume = createVolume( + "negative-capacity", -1, 0); + IllegalArgumentException exception = assertThrows( IllegalArgumentException.class, () -> DiskBalancerVolumeCalculation.getIdealUsage( Collections.singletonList( DiskBalancerVolumeCalculation.newVolumeFixedUsage( - zeroCapacityVolume, null)))); + negativeCapacityVolume, null)))); + + assertEquals("Negative capacity = -1: " + negativeCapacityVolume, + exception.getMessage()); + } + + @Test + void getIdealUsageRejectsNegativeEffectiveUsed() throws IOException { + HddsVolume volume = createVolume("negative-effective-used", 100, 100); + DiskBalancerVolumeCalculation.VolumeFixedUsage volumeUsage = + DiskBalancerVolumeCalculation.newVolumeFixedUsage( + volume, Collections.singletonMap(volume, -1L)); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> DiskBalancerVolumeCalculation.getIdealUsage( + Collections.singletonList(volumeUsage))); + + assertEquals("Negative effective used = " + volumeUsage.getEffectiveUsed() + + ": " + volume, exception.getMessage()); + } + + @Test + void getIdealUsageRejectsEffectiveUsedGreaterThanCapacity() + throws IOException { + HddsVolume volume = createVolume("effective-used-exceeds-capacity", 100, 0); + DiskBalancerVolumeCalculation.VolumeFixedUsage volumeUsage = + DiskBalancerVolumeCalculation.newVolumeFixedUsage( + volume, Collections.singletonMap(volume, 1L)); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> DiskBalancerVolumeCalculation.getIdealUsage( + Collections.singletonList(volumeUsage))); - assertEquals("total capacity must be greater than 0", exception.getMessage()); + assertEquals("Effective used = " + volumeUsage.getEffectiveUsed() + + " > capacity = " + volumeUsage.getUsage().getCapacity() + ": " + + volume, exception.getMessage()); } private HddsVolume createVolume(String name, long capacity, long available)