HDDS-10720. Datanode volume DU reserved percent should have a non-zero default value.

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 =

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);
   }
-
 }

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(

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);
   }
 }

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);