From 89b700d5bfe7763dd1adea09cce17bc7694ea9a9 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" <6454655+adoroszlai@users.noreply.github.com> Date: Mon, 18 Mar 2024 16:42:13 +0100 Subject: [PATCH] HDDS-10505. Move space reservation logic to VolumeUsage (#6370) --- .../hadoop/hdds/fs/SpaceUsageSource.java | 4 +- .../hadoop/hdds/fs/MockSpaceUsageSource.java | 22 ++++ .../container/common/impl/HddsDispatcher.java | 2 +- .../common/volume/StorageVolume.java | 5 - .../container/common/volume/VolumeInfo.java | 76 +---------- .../container/common/volume/VolumeUsage.java | 119 +++++++++++++----- .../common/impl/TestHddsDispatcher.java | 6 +- .../volume/TestReservedVolumeSpace.java | 28 ++--- 8 files changed, 134 insertions(+), 128 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageSource.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageSource.java index c25c0a40c53..a367cfbdc06 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageSource.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageSource.java @@ -57,9 +57,9 @@ final class Fixed implements SpaceUsageSource { private final long available; private final long used; - Fixed(long capacity, long available, long used) { + public Fixed(long capacity, long available, long used) { this.capacity = capacity; - this.available = available; + this.available = Math.max(Math.min(available, capacity - used), 0); this.used = used; } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/fs/MockSpaceUsageSource.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/fs/MockSpaceUsageSource.java index 76b6a0db89b..b20ce53597e 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/fs/MockSpaceUsageSource.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/fs/MockSpaceUsageSource.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hdds.fs; +import java.util.concurrent.atomic.AtomicLong; + /** * {@link SpaceUsageSource} implementations for testing. */ @@ -35,6 +37,26 @@ public static SpaceUsageSource fixed(long capacity, long available, return new SpaceUsageSource.Fixed(capacity, available, used); } + /** @return {@code SpaceUsageSource} with fixed capacity and dynamic usage */ + public static SpaceUsageSource of(long capacity, AtomicLong used) { + return new SpaceUsageSource() { + @Override + public long getUsedSpace() { + return used.get(); + } + + @Override + public long getCapacity() { + return capacity; + } + + @Override + public long getAvailable() { + return getCapacity() - getUsedSpace(); + } + }; + } + private MockSpaceUsageSource() { throw new UnsupportedOperationException("no instances"); } 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 321baadc309..12910b40209 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 @@ -614,7 +614,7 @@ private boolean isVolumeFull(Container container) { long volumeCapacity = precomputedVolumeSpace.getCapacity(); long volumeFreeSpaceToSpare = VolumeUsage.getMinVolumeFreeSpace(conf, volumeCapacity); - long volumeFree = volume.getAvailable(precomputedVolumeSpace); + long volumeFree = precomputedVolumeSpace.getAvailable(); long volumeCommitted = volume.getCommittedBytes(); long volumeAvailable = volumeFree - volumeCommitted; return (volumeAvailable <= volumeFreeSpaceToSpare); 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 d9d5a667b30..b85ac15c54e 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 @@ -456,11 +456,6 @@ public long getAvailable() { } - public long getAvailable(SpaceUsageSource precomputedVolumeSpace) { - return volumeInfo.map(info -> info.getAvailable(precomputedVolumeSpace)) - .orElse(0L); - } - public SpaceUsageSource getCurrentUsage() { return volumeInfo.map(VolumeInfo::getCurrentUsage) .orElse(SpaceUsageSource.UNKNOWN); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java index 0fde0903c2a..af890269255 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java @@ -20,11 +20,9 @@ import java.io.File; import java.io.IOException; -import java.util.Collection; import org.apache.hadoop.fs.StorageType; import org.apache.hadoop.hdds.conf.ConfigurationSource; -import org.apache.hadoop.hdds.conf.StorageSize; import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory; import org.apache.hadoop.hdds.fs.SpaceUsageCheckParams; @@ -33,10 +31,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT; - /** * Stores information about a disk/volume. * @@ -100,8 +94,6 @@ public final class VolumeInfo { // Space usage calculator private final VolumeUsage usage; - private long reservedInBytes; - /** * Builder for VolumeInfo. */ @@ -131,55 +123,6 @@ public VolumeInfo build() throws IOException { } } - private long getReserved(ConfigurationSource conf) { - if (conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT) - && conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED)) { - LOG.error("Both {} and {} are set. Set either one, not both. If the " + - "volume matches with volume parameter in former config, it is set " + - "as reserved space. If not it fall backs to the latter config.", - HDDS_DATANODE_DIR_DU_RESERVED, HDDS_DATANODE_DIR_DU_RESERVED_PERCENT); - } - - // 1. If hdds.datanode.dir.du.reserved is set for a volume then make it - // as the reserved bytes. - Collection reserveList = conf.getTrimmedStringCollection( - HDDS_DATANODE_DIR_DU_RESERVED); - for (String reserve : reserveList) { - String[] words = reserve.split(":"); - if (words.length < 2) { - LOG.error("Reserved space should config in pair, but current is {}", - reserve); - continue; - } - - if (words[0].trim().equals(rootDir)) { - try { - StorageSize size = StorageSize.parse(words[1].trim()); - return (long) size.getUnit().toBytes(size.getValue()); - } catch (Exception e) { - LOG.error("Failed to parse StorageSize: {}", words[1].trim(), e); - break; - } - } - } - - // 2. If hdds.datanode.dir.du.reserved not set and - // hdds.datanode.dir.du.reserved.percent is set, fall back to this config. - if (conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT)) { - float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, - HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); - if (0 <= percentage && percentage <= 1) { - return (long) Math.ceil(this.usage.getCapacity() * percentage); - } - //If it comes here then the percentage is not between 0-1. - LOG.error("The value of {} should be between 0 to 1. Defaulting to 0.", - HDDS_DATANODE_DIR_DU_RESERVED_PERCENT); - } - - //Both configs are not set, return 0. - return 0; - } - private VolumeInfo(Builder b) throws IOException { this.rootDir = b.rootDir; @@ -202,13 +145,11 @@ private VolumeInfo(Builder b) throws IOException { SpaceUsageCheckParams checkParams = usageCheckFactory.paramsFor(root); - this.usage = new VolumeUsage(checkParams); - this.reservedInBytes = getReserved(b.conf); - this.usage.setReserved(reservedInBytes); + usage = new VolumeUsage(checkParams, b.conf); } public long getCapacity() { - return Math.max(usage.getCapacity() - reservedInBytes, 0); + return usage.getCapacity(); } /** @@ -219,17 +160,11 @@ public long getCapacity() { * A) avail = capacity - used */ public long getAvailable() { - long avail = getCapacity() - usage.getUsedSpace(); - return Math.max(Math.min(avail, usage.getAvailable()), 0); - } - - public long getAvailable(SpaceUsageSource precomputedValues) { - long avail = precomputedValues.getCapacity() - usage.getUsedSpace(); - return Math.max(Math.min(avail, usage.getAvailable(precomputedValues)), 0); + return usage.getAvailable(); } public SpaceUsageSource getCurrentUsage() { - return usage.snapshot(); + return usage.getCurrentUsage(); } public void incrementUsedSpace(long usedSpace) { @@ -268,8 +203,7 @@ public VolumeUsage getUsageForTesting() { return usage; } - @VisibleForTesting public long getReservedInBytes() { - return reservedInBytes; + return usage.getReservedBytes(); } } 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 eafce1b4c43..be86cdaeadf 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 @@ -18,17 +18,25 @@ package org.apache.hadoop.ozone.container.common.volume; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.conf.StorageSize; import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.fs.CachingSpaceUsageSource; import org.apache.hadoop.hdds.fs.SpaceUsageCheckParams; import org.apache.hadoop.hdds.fs.SpaceUsageSource; +import org.apache.ratis.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Collection; + import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_DEFAULT; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT; /** * Class that wraps the space df of the Datanode Volumes used by SCM @@ -38,17 +46,32 @@ public class VolumeUsage { private final CachingSpaceUsageSource source; private boolean shutdownComplete; - private long reservedInBytes; + private final long reservedInBytes; private static final Logger LOG = LoggerFactory.getLogger(VolumeUsage.class); - VolumeUsage(SpaceUsageCheckParams checkParams) { + VolumeUsage(SpaceUsageCheckParams checkParams, ConfigurationSource conf) { source = new CachingSpaceUsageSource(checkParams); + reservedInBytes = getReserved(conf, checkParams.getPath(), source.getCapacity()); + Preconditions.assertTrue(reservedInBytes >= 0, reservedInBytes + " < 0"); start(); // TODO should start only on demand } + @VisibleForTesting + SpaceUsageSource realUsage() { + return source.snapshot(); + } + public long getCapacity() { - return Math.max(source.getCapacity(), 0); + return getCurrentUsage().getCapacity(); + } + + public long getAvailable() { + return getCurrentUsage().getAvailable(); + } + + public long getUsedSpace() { + return getCurrentUsage().getUsedSpace(); } /** @@ -59,21 +82,15 @@ public long getCapacity() { * remainingReserved * B) avail = fsAvail - Max(reserved - other, 0); */ - public long getAvailable() { - return source.getAvailable() - getRemainingReserved(); - } - - public long getAvailable(SpaceUsageSource precomputedVolumeSpace) { - long available = precomputedVolumeSpace.getAvailable(); - return available - getRemainingReserved(precomputedVolumeSpace); - } - - public long getUsedSpace() { - return source.getUsedSpace(); - } + public SpaceUsageSource getCurrentUsage() { + SpaceUsageSource real = realUsage(); - public SpaceUsageSource snapshot() { - return source.snapshot(); + return reservedInBytes == 0 + ? real + : new SpaceUsageSource.Fixed( + Math.max(real.getCapacity() - reservedInBytes, 0), + Math.max(real.getAvailable() - getRemainingReserved(real), 0), + real.getUsedSpace()); } public void incrementUsedSpace(long usedSpace) { @@ -90,19 +107,10 @@ public void decrementUsedSpace(long reclaimedSpace) { * so there could be that DU value > totalUsed when there are deletes. * @return other used space */ - private long getOtherUsed() { - long totalUsed = source.getCapacity() - source.getAvailable(); - return Math.max(totalUsed - source.getUsedSpace(), 0L); - } - - private long getOtherUsed(SpaceUsageSource precomputedVolumeSpace) { + private static long getOtherUsed(SpaceUsageSource precomputedVolumeSpace) { long totalUsed = precomputedVolumeSpace.getCapacity() - precomputedVolumeSpace.getAvailable(); - return Math.max(totalUsed - source.getUsedSpace(), 0L); - } - - private long getRemainingReserved() { - return Math.max(reservedInBytes - getOtherUsed(), 0L); + return Math.max(totalUsed - precomputedVolumeSpace.getUsedSpace(), 0L); } private long getRemainingReserved( @@ -125,8 +133,8 @@ public void refreshNow() { source.refreshNow(); } - public void setReserved(long reserved) { - this.reservedInBytes = reserved; + public long getReservedBytes() { + return reservedInBytes; } /** @@ -170,4 +178,55 @@ public static boolean hasVolumeEnoughSpace(long volumeAvailableSpace, return (volumeAvailableSpace - volumeCommittedBytesCount) > Math.max(requiredSpace, volumeFreeSpaceToSpare); } + + private static long getReserved(ConfigurationSource conf, String rootDir, + long capacity) { + if (conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT) + && conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED)) { + LOG.error("Both {} and {} are set. Set either one, not both. If the " + + "volume matches with volume parameter in former config, it is set " + + "as reserved space. If not it fall backs to the latter config.", + HDDS_DATANODE_DIR_DU_RESERVED, HDDS_DATANODE_DIR_DU_RESERVED_PERCENT); + } + + // 1. If hdds.datanode.dir.du.reserved is set for a volume then make it + // as the reserved bytes. + Collection reserveList = conf.getTrimmedStringCollection( + HDDS_DATANODE_DIR_DU_RESERVED); + for (String reserve : reserveList) { + String[] words = reserve.split(":"); + if (words.length < 2) { + LOG.error("Reserved space should config in pair, but current is {}", + reserve); + continue; + } + + if (words[0].trim().equals(rootDir)) { + try { + StorageSize size = StorageSize.parse(words[1].trim()); + return (long) size.getUnit().toBytes(size.getValue()); + } catch (Exception e) { + LOG.error("Failed to parse StorageSize: {}", words[1].trim(), e); + break; + } + } + } + + // 2. If hdds.datanode.dir.du.reserved not set and + // hdds.datanode.dir.du.reserved.percent is set, fall back to this config. + if (conf.isConfigured(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT)) { + float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, + HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); + if (0 <= percentage && percentage <= 1) { + return (long) Math.ceil(capacity * percentage); + } + //If it comes here then the percentage is not between 0-1. + LOG.error("The value of {} should be between 0 to 1. Defaulting to 0.", + HDDS_DATANODE_DIR_DU_RESERVED_PERCENT); + } + + //Both configs are not set, return 0. + return 0; + } + } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java index 95df6c647f8..14015202310 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java @@ -25,6 +25,7 @@ import org.apache.hadoop.hdds.client.BlockID; 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.SpaceUsageSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -78,7 +79,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.hadoop.hdds.fs.MockSpaceUsagePersistence.inMemory; -import static org.apache.hadoop.hdds.fs.MockSpaceUsageSource.fixed; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getContainerCommandResponse; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.COMMIT_STAGE; @@ -178,7 +178,8 @@ public void testContainerCloseActionWhenVolumeFull( .conf(conf).usageCheckFactory(MockSpaceUsageCheckFactory.NONE); // state of cluster : available (140) > 100 ,datanode volume // utilisation threshold not yet reached. container creates are successful. - SpaceUsageSource spaceUsage = fixed(500, 140, 360); + AtomicLong usedSpace = new AtomicLong(360); + SpaceUsageSource spaceUsage = MockSpaceUsageSource.of(500, usedSpace); SpaceUsageCheckFactory factory = MockSpaceUsageCheckFactory.of( spaceUsage, Duration.ZERO, inMemory(new AtomicLong(0))); @@ -212,6 +213,7 @@ public void testContainerCloseActionWhenVolumeFull( hddsDispatcher.setClusterId(scmId.toString()); containerData.getVolume().getVolumeInfo() .ifPresent(volumeInfo -> volumeInfo.incrementUsedSpace(50)); + usedSpace.addAndGet(50); ContainerCommandResponseProto response = hddsDispatcher .dispatch(getWriteChunkRequest(dd.getUuidString(), 1L, 1L), null); assertEquals(ContainerProtos.Result.SUCCESS, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java index c5d127446bf..2a7cae57dbf 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java @@ -66,20 +66,15 @@ public void testVolumeCapacityAfterReserve() throws Exception { HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); long volumeCapacity = hddsVolume.getCapacity(); - //Gets the actual total capacity - long totalCapacity = hddsVolume.getVolumeInfo().get() - .getUsageForTesting().getCapacity(); - long reservedCapacity = hddsVolume.getVolumeInfo().get() - .getReservedInBytes(); - //Volume Capacity with Reserved - long volumeCapacityReserved = totalCapacity - reservedCapacity; + VolumeUsage usage = hddsVolume.getVolumeInfo().get().getUsageForTesting(); - long reservedFromVolume = hddsVolume.getVolumeInfo().get() - .getReservedInBytes(); + //Gets the actual total capacity + long totalCapacity = usage.realUsage().getCapacity(); + long reservedCapacity = usage.getReservedBytes(); long reservedCalculated = (long) Math.ceil(totalCapacity * percentage); - assertEquals(reservedFromVolume, reservedCalculated); - assertEquals(volumeCapacity, volumeCapacityReserved); + assertEquals(reservedCalculated, reservedCapacity); + assertEquals(totalCapacity - reservedCapacity, volumeCapacity); } /** @@ -119,16 +114,15 @@ public void testFallbackToPercentConfig() throws Exception { temp.toString() + ":500B"); HddsVolume hddsVolume = volumeBuilder.conf(conf).build(); - long reservedFromVolume = hddsVolume.getVolumeInfo().get() - .getReservedInBytes(); - assertNotEquals(reservedFromVolume, 0); + VolumeUsage usage = hddsVolume.getVolumeInfo().get().getUsageForTesting(); + long reservedFromVolume = usage.getReservedBytes(); + assertNotEquals(0, reservedFromVolume); - long totalCapacity = hddsVolume.getVolumeInfo().get() - .getUsageForTesting().getCapacity(); + long totalCapacity = usage.realUsage().getCapacity(); float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); long reservedCalculated = (long) Math.ceil(totalCapacity * percentage); - assertEquals(reservedFromVolume, reservedCalculated); + assertEquals(reservedCalculated, reservedFromVolume); } @Test