From ab89fcdc0b27a96162038c1426dc6302393b7b1e Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 11:01:31 +0100 Subject: [PATCH 1/9] eliminate parameternumber warning in StorageLocationReport --- .../common/impl/StorageLocationReport.java | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java index 301a43320f38..2f91e16c1481 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java @@ -41,19 +41,16 @@ public final class StorageLocationReport implements private final StorageType storageType; private final String storageLocation; - @SuppressWarnings("checkstyle:parameternumber") - private StorageLocationReport(String id, boolean failed, long capacity, - long scmUsed, long remaining, long committed, long freeSpaceToSpare, - StorageType storageType, String storageLocation) { - this.id = id; - this.failed = failed; - this.capacity = capacity; - this.scmUsed = scmUsed; - this.remaining = remaining; - this.committed = committed; - this.freeSpaceToSpare = freeSpaceToSpare; - this.storageType = storageType; - this.storageLocation = storageLocation; + private StorageLocationReport(Builder builder) { + this.id = builder.id; + this.failed = builder.failed; + this.capacity = builder.capacity; + this.scmUsed = builder.scmUsed; + this.remaining = builder.remaining; + this.committed = builder.committed; + this.freeSpaceToSpare = builder.freeSpaceToSpare; + this.storageType = builder.storageType; + this.storageLocation = builder.storageLocation; } @Override @@ -387,8 +384,7 @@ public Builder setStorageLocation(String storageLocationValue) { * @return StorageLocationReport */ public StorageLocationReport build() { - return new StorageLocationReport(id, failed, capacity, scmUsed, - remaining, committed, freeSpaceToSpare, storageType, storageLocation); + return new StorageLocationReport(this); } } From f090ff0f8df434d8e200bfe3dded4de6e80bf64f Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 11:24:26 +0100 Subject: [PATCH 2/9] move StorageLocationReport creation to StorageVolume --- .../common/impl/StorageLocationReport.java | 9 +++ .../container/common/volume/HddsVolume.java | 11 ++++ .../common/volume/MutableVolumeSet.java | 66 ++----------------- .../common/volume/StorageVolume.java | 22 +++++++ 4 files changed, 47 insertions(+), 61 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java index 2f91e16c1481..25ab2017252b 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java @@ -301,6 +301,10 @@ public Builder setFailed(boolean failedValue) { return this; } + public boolean isFailed() { + return failed; + } + /** * Sets the capacity of volume. * @@ -311,6 +315,11 @@ public Builder setCapacity(long capacityValue) { this.capacity = capacityValue; return this; } + + public long getCapacity() { + return capacity; + } + /** * Sets the scmUsed Value. * diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java index b88961e04ae4..a81bde59b4fc 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java @@ -35,6 +35,7 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult; +import org.apache.hadoop.ozone.container.common.impl.StorageLocationReport; import org.apache.hadoop.ozone.container.common.utils.DatanodeStoreCache; import org.apache.hadoop.ozone.container.common.utils.HddsVolumeUtil; import org.apache.hadoop.ozone.container.common.utils.RawDB; @@ -179,6 +180,16 @@ public VolumeInfoMetrics getVolumeInfoStats() { return volumeInfoMetrics; } + @Override + protected StorageLocationReport.Builder reportBuilder() { + StorageLocationReport.Builder builder = super.reportBuilder(); + if (!builder.isFailed()) { + builder.setCommitted(getCommittedBytes()) + .setFreeSpaceToSpare(getFreeSpaceToSpare(builder.getCapacity())); + } + return builder; + } + @Override public void failVolume() { super.failVolume(); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java index 2b41c4872de2..9616eeeab9ab 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java @@ -21,20 +21,17 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.io.IOException; -import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collection; import java.util.EnumMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.hadoop.fs.StorageType; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory; -import org.apache.hadoop.hdds.fs.SpaceUsageSource; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.hdfs.server.datanode.StorageLocation; import org.apache.hadoop.ozone.container.common.impl.StorageLocationReport; @@ -455,68 +452,15 @@ public boolean hasEnoughVolumes() { } public StorageLocationReport[] getStorageReport() { - boolean failed; this.readLock(); try { - StorageLocationReport[] reports = new StorageLocationReport[volumeMap - .size() + failedVolumeMap.size()]; + StorageLocationReport[] reports = new StorageLocationReport[volumeMap.size() + failedVolumeMap.size()]; int counter = 0; - StorageVolume volume; - for (Map.Entry entry : volumeMap.entrySet()) { - volume = entry.getValue(); - Optional volumeUsage = volume.getVolumeUsage(); - long scmUsed = 0; - long remaining = 0; - long capacity = 0; - long committed = 0; - long spare = 0; - String rootDir = volume.getVolumeRootDir(); - failed = true; - if (volumeUsage.isPresent()) { - try { - SpaceUsageSource usage = volumeUsage.get().getCurrentUsage(); - scmUsed = usage.getUsedSpace(); - remaining = usage.getAvailable(); - capacity = usage.getCapacity(); - HddsVolume hddsVolume = volume instanceof HddsVolume ? (HddsVolume) volume : null; - committed = hddsVolume != null ? hddsVolume.getCommittedBytes() : 0; - spare = hddsVolume != null ? hddsVolume.getFreeSpaceToSpare(capacity) : 0; - failed = false; - } catch (UncheckedIOException ex) { - LOG.warn("Failed to get scmUsed and remaining for container " + - "storage location {}", rootDir, ex); - // reset scmUsed and remaining if df/du failed. - scmUsed = 0; - remaining = 0; - capacity = 0; - } - } - - StorageLocationReport.Builder builder = - StorageLocationReport.newBuilder(); - builder.setStorageLocation(rootDir) - .setId(volume.getStorageID()) - .setFailed(failed) - .setCapacity(capacity) - .setRemaining(remaining) - .setScmUsed(scmUsed) - .setCommitted(committed) - .setFreeSpaceToSpare(spare) - .setStorageType(volume.getStorageType()); - StorageLocationReport r = builder.build(); - reports[counter++] = r; + for (StorageVolume volume : volumeMap.values()) { + reports[counter++] = volume.getReport(); } - for (Map.Entry entry - : failedVolumeMap.entrySet()) { - volume = entry.getValue(); - StorageLocationReport.Builder builder = StorageLocationReport - .newBuilder(); - builder.setStorageLocation(volume.getStorageDir() - .getAbsolutePath()).setId(volume.getStorageID()).setFailed(true) - .setCapacity(0).setRemaining(0).setScmUsed(0).setStorageType( - volume.getStorageType()); - StorageLocationReport r = builder.build(); - reports[counter++] = r; + for (StorageVolume volume : failedVolumeMap.values()) { + reports[counter++] = volume.getReport(); } return reports; } finally { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java index 318fa0ab9df0..918a5d36d84f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult; import org.apache.hadoop.ozone.common.InconsistentStorageStateException; import org.apache.hadoop.ozone.container.common.helpers.DatanodeVersionFile; +import org.apache.hadoop.ozone.container.common.impl.StorageLocationReport; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.hadoop.ozone.container.common.utils.DiskCheckUtil; import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil; @@ -454,6 +455,27 @@ public SpaceUsageSource getCurrentUsage() { .orElse(SpaceUsageSource.UNKNOWN); } + protected StorageLocationReport.Builder reportBuilder() { + StorageLocationReport.Builder builder = StorageLocationReport.newBuilder() + .setFailed(isFailed()) + .setId(getStorageID()) + .setStorageLocation(volumeRoot) + .setStorageType(storageType); + + if (!builder.isFailed()) { + SpaceUsageSource usage = getCurrentUsage(); + builder.setCapacity(usage.getCapacity()) + .setRemaining(usage.getAvailable()) + .setScmUsed(usage.getUsedSpace()); + } + + return builder; + } + + public StorageLocationReport getReport() { + return reportBuilder().build(); + } + public File getStorageDir() { return this.storageDir; } From cc9ba5802b80d2147c0842b94a16c171e414961d Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 12:06:52 +0100 Subject: [PATCH 3/9] use storage report for deciding hasVolumeEnoughSpace --- .../container/common/impl/HddsDispatcher.java | 6 +----- .../common/volume/AvailableSpaceFilter.java | 16 +++++----------- .../container/common/volume/VolumeUsage.java | 12 ++++++++++++ .../hdds/scm/SCMCommonPlacementPolicy.java | 4 +--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index 6fdbcb1b3cd8..846bc1ae0c89 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -36,7 +36,6 @@ import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.conf.ConfigurationSource; -import org.apache.hadoop.hdds.fs.SpaceUsageSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto; @@ -616,10 +615,7 @@ private boolean isVolumeFull(Container container) { .orElse(Boolean.FALSE); if (isOpen) { HddsVolume volume = container.getContainerData().getVolume(); - SpaceUsageSource usage = volume.getCurrentUsage(); - long volumeFreeSpaceToSpare = volume.getFreeSpaceToSpare(usage.getCapacity()); - return !VolumeUsage.hasVolumeEnoughSpace(usage.getAvailable(), volume.getCommittedBytes(), 0, - volumeFreeSpaceToSpare); + return !VolumeUsage.hasVolumeEnoughSpace(volume.getReport(), 0); } return false; } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java index d584600acf63..1c995e0503f3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java @@ -20,7 +20,7 @@ import java.util.HashMap; import java.util.Map; import java.util.function.Predicate; -import org.apache.hadoop.hdds.fs.SpaceUsageSource; +import org.apache.hadoop.ozone.container.common.impl.StorageLocationReport; /** * Filter for selecting volumes with enough space for a new container. @@ -39,19 +39,13 @@ public class AvailableSpaceFilter implements Predicate { @Override public boolean test(HddsVolume vol) { - SpaceUsageSource usage = vol.getCurrentUsage(); - long volumeCapacity = usage.getCapacity(); - long free = usage.getAvailable(); - long committed = vol.getCommittedBytes(); - long available = free - committed; - long volumeFreeSpaceToSpare = vol.getFreeSpaceToSpare(volumeCapacity); - boolean hasEnoughSpace = VolumeUsage.hasVolumeEnoughSpace(free, committed, - requiredSpace, volumeFreeSpaceToSpare); + StorageLocationReport report = vol.getReport(); + boolean hasEnoughSpace = VolumeUsage.hasVolumeEnoughSpace(report, requiredSpace); - mostAvailableSpace = Math.max(available, mostAvailableSpace); + mostAvailableSpace = Math.max(report.getRemaining(), mostAvailableSpace); if (!hasEnoughSpace) { - fullVolumes.put(vol, new AvailableSpace(free, committed)); + fullVolumes.put(vol, new AvailableSpace(report.getRemaining(), report.getCommitted())); } return hasEnoughSpace; 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 0fcab327a4d7..ba5e930cb4ac 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 @@ -31,6 +31,8 @@ import org.apache.hadoop.hdds.fs.CachingSpaceUsageSource; import org.apache.hadoop.hdds.fs.SpaceUsageCheckParams; import org.apache.hadoop.hdds.fs.SpaceUsageSource; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; +import org.apache.hadoop.ozone.container.common.impl.StorageLocationReport; import org.apache.ratis.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -184,6 +186,16 @@ public static boolean hasVolumeEnoughSpace(long volumeAvailableSpace, return (volumeAvailableSpace - volumeCommittedBytesCount - volumeFreeSpaceToSpare) > requiredSpace; } + public static boolean hasVolumeEnoughSpace(StorageReportProto report, long requiredSpace) { + return hasVolumeEnoughSpace(report.getRemaining(), report.getCommitted(), requiredSpace, + report.getFreeSpaceToSpare()); + } + + public static boolean hasVolumeEnoughSpace(StorageLocationReport report, long requiredSpace) { + return hasVolumeEnoughSpace(report.getRemaining(), report.getCommitted(), requiredSpace, + report.getFreeSpaceToSpare()); + } + private static long getReserved(ConfigurationSource conf, String rootDir, long capacity) { if (conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 2c46c0e4e26e..1158aadf6082 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -309,9 +309,7 @@ public static boolean hasEnoughSpace(DatanodeDetails datanodeDetails, if (dataSizeRequired > 0) { for (StorageReportProto reportProto : datanodeInfo.getStorageReports()) { - if (VolumeUsage.hasVolumeEnoughSpace(reportProto.getRemaining(), - reportProto.getCommitted(), dataSizeRequired, - reportProto.getFreeSpaceToSpare())) { + if (VolumeUsage.hasVolumeEnoughSpace(reportProto, dataSizeRequired)) { enoughForData = true; break; } From be95f71305609bd1b34c6d7f5ea17b149d59e3dc Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 12:14:51 +0100 Subject: [PATCH 4/9] mark StorageLocationReport as Immutable --- .../ozone/container/common/impl/StorageLocationReport.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java index 25ab2017252b..022863c2a772 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/StorageLocationReport.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.container.common.impl; import java.io.IOException; +import net.jcip.annotations.Immutable; import org.apache.hadoop.fs.StorageType; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; @@ -28,8 +29,8 @@ * Storage location stats of datanodes that provide back store for containers. * */ -public final class StorageLocationReport implements - StorageLocationReportMXBean { +@Immutable +public final class StorageLocationReport implements StorageLocationReportMXBean { private final String id; private final boolean failed; From 2d5284bb22cf453967b2683dc74895383b4265ab Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 12:17:04 +0100 Subject: [PATCH 5/9] make hasVolumeEnoughSpace(long, long, long, long) private --- .../hadoop/ozone/container/common/volume/VolumeUsage.java | 8 +++----- 1 file changed, 3 insertions(+), 5 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 ba5e930cb4ac..8a0435a03f66 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 @@ -179,11 +179,9 @@ public long getReservedInBytes() { return reservedInBytes; } - public static boolean hasVolumeEnoughSpace(long volumeAvailableSpace, - long volumeCommittedBytesCount, - long requiredSpace, - long volumeFreeSpaceToSpare) { - return (volumeAvailableSpace - volumeCommittedBytesCount - volumeFreeSpaceToSpare) > requiredSpace; + private static boolean hasVolumeEnoughSpace( + long available, long committed, long required, long minFreeSpace) { + return (available - committed - minFreeSpace) > required; } public static boolean hasVolumeEnoughSpace(StorageReportProto report, long requiredSpace) { From f19ae5da17494f394e32fdbf9702993d5a4ba659 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 12:17:54 +0100 Subject: [PATCH 6/9] move parameter required to first position --- .../ozone/container/common/volume/VolumeUsage.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 8a0435a03f66..51e54296d31b 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 @@ -180,18 +180,18 @@ public long getReservedInBytes() { } private static boolean hasVolumeEnoughSpace( - long available, long committed, long required, long minFreeSpace) { + long required, long available, long committed, long minFreeSpace) { return (available - committed - minFreeSpace) > required; } public static boolean hasVolumeEnoughSpace(StorageReportProto report, long requiredSpace) { - return hasVolumeEnoughSpace(report.getRemaining(), report.getCommitted(), requiredSpace, - report.getFreeSpaceToSpare()); + return hasVolumeEnoughSpace(requiredSpace, + report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); } public static boolean hasVolumeEnoughSpace(StorageLocationReport report, long requiredSpace) { - return hasVolumeEnoughSpace(report.getRemaining(), report.getCommitted(), requiredSpace, - report.getFreeSpaceToSpare()); + return hasVolumeEnoughSpace(requiredSpace, + report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); } private static long getReserved(ConfigurationSource conf, String rootDir, From 8a4024ef5052350be4aab92872c68c1eae8839db Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 26 Mar 2025 13:33:19 +0100 Subject: [PATCH 7/9] fix AvailableSpaceFilter --- .../container/common/volume/AvailableSpaceFilter.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java index 1c995e0503f3..c969d7bb8eda 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java @@ -40,12 +40,15 @@ public class AvailableSpaceFilter implements Predicate { @Override public boolean test(HddsVolume vol) { StorageLocationReport report = vol.getReport(); + long free = report.getRemaining(); + long committed = vol.getCommittedBytes(); + long available = free - committed; boolean hasEnoughSpace = VolumeUsage.hasVolumeEnoughSpace(report, requiredSpace); - mostAvailableSpace = Math.max(report.getRemaining(), mostAvailableSpace); + mostAvailableSpace = Math.max(available, mostAvailableSpace); if (!hasEnoughSpace) { - fullVolumes.put(vol, new AvailableSpace(report.getRemaining(), report.getCommitted())); + fullVolumes.put(vol, new AvailableSpace(free, committed)); } return hasEnoughSpace; From 86995d8ab4c029a3accf2bc012cab8fae79f393b Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Thu, 27 Mar 2025 07:39:47 +0100 Subject: [PATCH 8/9] hasVolumeEnoughSpace -> getUsableSpace (without requiredSpace param) --- .../container/common/impl/HddsDispatcher.java | 2 +- .../common/volume/AvailableSpaceFilter.java | 20 ++++++++----------- .../container/common/volume/VolumeUsage.java | 16 +++++++-------- .../hdds/scm/SCMCommonPlacementPolicy.java | 2 +- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index 846bc1ae0c89..669fcb367b8a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -615,7 +615,7 @@ private boolean isVolumeFull(Container container) { .orElse(Boolean.FALSE); if (isOpen) { HddsVolume volume = container.getContainerData().getVolume(); - return !VolumeUsage.hasVolumeEnoughSpace(volume.getReport(), 0); + return VolumeUsage.getUsableSpace(volume.getReport()) > 0; } return false; } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java index 3d58a487e814..ef50e73cb314 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/AvailableSpaceFilter.java @@ -40,15 +40,13 @@ public AvailableSpaceFilter(long requiredSpace) { @Override public boolean test(HddsVolume vol) { StorageLocationReport report = vol.getReport(); - long free = report.getRemaining(); - long committed = vol.getCommittedBytes(); - long available = free - committed; - boolean hasEnoughSpace = VolumeUsage.hasVolumeEnoughSpace(report, requiredSpace); + long available = VolumeUsage.getUsableSpace(report); + boolean hasEnoughSpace = available > requiredSpace; mostAvailableSpace = Math.max(available, mostAvailableSpace); if (!hasEnoughSpace) { - fullVolumes.put(vol, new AvailableSpace(free, committed)); + fullVolumes.put(vol, new AvailableSpace(report)); } return hasEnoughSpace; @@ -69,18 +67,16 @@ public String toString() { } private static class AvailableSpace { - private final long free; - private final long committed; + private final StorageLocationReport report; - AvailableSpace(long free, long committed) { - this.free = free; - this.committed = committed; + AvailableSpace(StorageLocationReport report) { + this.report = report; } @Override public String toString() { - return "free: " + free + - ", committed: " + committed; + return "free: " + report.getRemaining() + + ", committed: " + report.getCommitted(); } } } 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 51e54296d31b..84f04ec87149 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 @@ -179,19 +179,17 @@ public long getReservedInBytes() { return reservedInBytes; } - private static boolean hasVolumeEnoughSpace( - long required, long available, long committed, long minFreeSpace) { - return (available - committed - minFreeSpace) > required; + private static long getUsableSpace( + long available, long committed, long minFreeSpace) { + return available - committed - minFreeSpace; } - public static boolean hasVolumeEnoughSpace(StorageReportProto report, long requiredSpace) { - return hasVolumeEnoughSpace(requiredSpace, - report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); + public static long getUsableSpace(StorageReportProto report) { + return getUsableSpace(report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); } - public static boolean hasVolumeEnoughSpace(StorageLocationReport report, long requiredSpace) { - return hasVolumeEnoughSpace(requiredSpace, - report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); + public static long getUsableSpace(StorageLocationReport report) { + return getUsableSpace(report.getRemaining(), report.getCommitted(), report.getFreeSpaceToSpare()); } private static long getReserved(ConfigurationSource conf, String rootDir, diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 1158aadf6082..6dcfcaa9f47c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -309,7 +309,7 @@ public static boolean hasEnoughSpace(DatanodeDetails datanodeDetails, if (dataSizeRequired > 0) { for (StorageReportProto reportProto : datanodeInfo.getStorageReports()) { - if (VolumeUsage.hasVolumeEnoughSpace(reportProto, dataSizeRequired)) { + if (VolumeUsage.getUsableSpace(reportProto) > dataSizeRequired) { enoughForData = true; break; } From e71eaae951b027c16d18d85dcf0da107446118f5 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Thu, 27 Mar 2025 08:57:02 +0100 Subject: [PATCH 9/9] fix wrong comparison for isVolumeFull --- .../hadoop/ozone/container/common/impl/HddsDispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index 669fcb367b8a..a8a73c783887 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -615,7 +615,7 @@ private boolean isVolumeFull(Container container) { .orElse(Boolean.FALSE); if (isOpen) { HddsVolume volume = container.getContainerData().getVolume(); - return VolumeUsage.getUsableSpace(volume.getReport()) > 0; + return VolumeUsage.getUsableSpace(volume.getReport()) <= 0; } return false; }