diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java index caf0127d213..1d175197b1c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java @@ -223,7 +223,7 @@ public final class ScmConfigKeys { "hdds.datanode.dir.du.reserved"; public static final String HDDS_DATANODE_DIR_DU_RESERVED_PERCENT = "hdds.datanode.dir.du.reserved.percent"; - public static final float HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT = 0; + public static final float HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT = 0.0001f; public static final String OZONE_SCM_HANDLER_COUNT_KEY = "ozone.scm.handler.count.key"; public static final String OZONE_SCM_CLIENT_HANDLER_COUNT_KEY = 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 be86cdaeadf..d18998821b1 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 @@ -29,6 +29,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; +import java.io.IOException; import java.util.Collection; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE; @@ -196,37 +198,34 @@ private static long getReserved(ConfigurationSource conf, String rootDir, for (String reserve : reserveList) { String[] words = reserve.split(":"); if (words.length < 2) { - LOG.error("Reserved space should config in pair, but current is {}", + LOG.error("Reserved space should be configured in a pair, but current value is {}", reserve); continue; } - if (words[0].trim().equals(rootDir)) { - try { + try { + String path = new File(words[0]).getCanonicalPath(); + if (path.equals(rootDir)) { 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; } + } catch (IllegalArgumentException e) { + LOG.error("Failed to parse StorageSize {} from config {}", words[1].trim(), HDDS_DATANODE_DIR_DU_RESERVED, e); + } catch (IOException e) { + LOG.error("Failed to read storage path {} from config {}", words[1].trim(), HDDS_DATANODE_DIR_DU_RESERVED, e); } } - // 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); + // 2. If hdds.datanode.dir.du.reserved not set then fall back to hdds.datanode.dir.du.reserved.percent, using + // either its set value or default value if it has not been set. + float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, + HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); + if (percentage < 0 || percentage > 1) { + LOG.error("The value of {} should be between 0 to 1. Falling back to default value {}", + HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); + percentage = HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT; } - //Both configs are not set, return 0. - return 0; + return (long) Math.ceil(capacity * percentage); } - } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestCapacityVolumeChoosingPolicy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestCapacityVolumeChoosingPolicy.java index 4718df3ae3f..1eba25c3571 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestCapacityVolumeChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestCapacityVolumeChoosingPolicy.java @@ -38,6 +38,7 @@ import java.util.Map; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DATANODE_VOLUME_CHOOSING_POLICY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -60,6 +61,9 @@ public void setup() throws Exception { String volume2 = baseDir + "disk2"; String volume3 = baseDir + "disk3"; policy = new CapacityVolumeChoosingPolicy(); + // Use the exact capacity and availability specified in this test. Do not reserve space to prevent volumes from + // filling up. + CONF.setFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, 0); SpaceUsageSource source1 = MockSpaceUsageSource.fixed(500, 100); SpaceUsageCheckFactory factory1 = MockSpaceUsageCheckFactory.of( 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 2a7cae57dbf..cd9beab4b79 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 @@ -25,6 +25,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.io.File; +import java.nio.file.Files; import java.nio.file.Path; import java.util.UUID; @@ -52,6 +54,27 @@ public void setup() throws Exception { .usageCheckFactory(MockSpaceUsageCheckFactory.NONE); } + @Test + public void testDefaultConfig() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + HddsVolume hddsVolume = volumeBuilder.conf(conf).build(); + float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, + HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); + assertEquals(percentage, HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); + + // Gets the total capacity reported by Ozone, which may be limited to less than the volume's real capacity by the + // DU reserved configurations. + long volumeCapacity = hddsVolume.getCapacity(); + VolumeUsage usage = hddsVolume.getVolumeInfo().get().getUsageForTesting(); + + // Gets the actual total capacity without accounting for DU reserved space configurations. + long totalCapacity = usage.realUsage().getCapacity(); + long reservedCapacity = usage.getReservedBytes(); + + assertEquals(getExpectedDefaultReserved(hddsVolume), reservedCapacity); + assertEquals(totalCapacity - reservedCapacity, volumeCapacity); + } + /** * Test reserved capacity with respect to the percentage of actual capacity. * @throws Exception @@ -92,17 +115,7 @@ public void testReservedWhenBothConfigSet() throws Exception { long reservedFromVolume = hddsVolume.getVolumeInfo().get() .getReservedInBytes(); - assertEquals(reservedFromVolume, 500); - } - - @Test - public void testReservedToZeroWhenBothConfigNotSet() throws Exception { - OzoneConfiguration conf = new OzoneConfiguration(); - HddsVolume hddsVolume = volumeBuilder.conf(conf).build(); - - long reservedFromVolume = hddsVolume.getVolumeInfo().get() - .getReservedInBytes(); - assertEquals(reservedFromVolume, 0); + assertEquals(500, reservedFromVolume); } @Test @@ -136,7 +149,7 @@ public void testInvalidConfig() throws Exception { long reservedFromVolume1 = hddsVolume1.getVolumeInfo().get() .getReservedInBytes(); - assertEquals(reservedFromVolume1, 0); + assertEquals(getExpectedDefaultReserved(hddsVolume1), reservedFromVolume1); OzoneConfiguration conf2 = new OzoneConfiguration(); @@ -146,6 +159,27 @@ public void testInvalidConfig() throws Exception { long reservedFromVolume2 = hddsVolume2.getVolumeInfo().get() .getReservedInBytes(); - assertEquals(reservedFromVolume2, 0); + assertEquals(getExpectedDefaultReserved(hddsVolume2), reservedFromVolume2); + } + + @Test + public void testPathsCanonicalized() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + + // Create symlink in folder (which is the root of the volume) + Path symlink = new File(temp.toFile(), "link").toPath(); + Files.createSymbolicLink(symlink, folder); + + // Use the symlink in the configuration. Canonicalization should still match it to folder used in the volume config. + conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED, symlink + ":500B"); + HddsVolume hddsVolume = volumeBuilder.conf(conf).build(); + + long reservedFromVolume = hddsVolume.getVolumeInfo().get().getReservedInBytes(); + assertEquals(500, reservedFromVolume); + } + + private long getExpectedDefaultReserved(HddsVolume volume) { + long totalCapacity = volume.getVolumeInfo().get().getUsageForTesting().realUsage().getCapacity(); + return (long) Math.ceil(totalCapacity * HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT); } } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestRoundRobinVolumeChoosingPolicy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestRoundRobinVolumeChoosingPolicy.java index cc6fe87e19d..1df26365531 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestRoundRobinVolumeChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestRoundRobinVolumeChoosingPolicy.java @@ -36,6 +36,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -58,6 +59,10 @@ public void setup() throws Exception { String volume2 = baseDir + "disk2"; policy = new RoundRobinVolumeChoosingPolicy(); + // Use the exact capacity and availability specified in this test. Do not reserve space to prevent volumes from + // filling up. + CONF.setFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, 0); + SpaceUsageSource source1 = MockSpaceUsageSource.fixed(500, 100); SpaceUsageCheckFactory factory1 = MockSpaceUsageCheckFactory.of( source1, Duration.ZERO, SpaceUsagePersistence.None.INSTANCE);