HDDS-12676. Prefer minFreeSpace if minFreeSpacePercent is also defined

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java

@@ -73,10 +73,17 @@ public class DatanodeConfiguration extends ReconfigurableConfig {
       "hdds.datanode.disk.check.min.gap";
   public static final String DISK_CHECK_TIMEOUT_KEY =
       "hdds.datanode.disk.check.timeout";
+
+  // Minimum space should be left on volume.
+  // Ex: If volume has 1000GB and minFreeSpace is configured as 10GB,
+  // In this case when availableSpace is 10GB or below, volume is assumed as full
   public static final String HDDS_DATANODE_VOLUME_MIN_FREE_SPACE =
       "hdds.datanode.volume.min.free.space";
   public static final String HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_DEFAULT =
       "5GB";
+  // Minimum percent of space should be left on volume.
+  // Ex: If volume has 1000GB and minFreeSpacePercent is configured as 2%,
+  // In this case when availableSpace is 20GB(2% of 1000) or below, volume is assumed as full
   public static final String HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT =
       "hdds.datanode.volume.min.free.space.percent";
   static final byte MIN_FREE_SPACE_UNSET = -1;
@@ -338,15 +345,22 @@ public void setBlockDeletionLimit(int limit) {
       description = "This determines the free space to be used for closing containers" +
           " When the difference between volume capacity and used reaches this number," +
           " containers that reside on this volume will be closed and no new containers" +
-          " would be allocated on this volume."
+          " would be allocated on this volume." +
+          " Either of min.free.space or min.free.space.percent should be configured, when both are set then" +
+          " min.free.space will be used."
   )
   private long minFreeSpace = MIN_FREE_SPACE_UNSET;
 
   @Config(key = "hdds.datanode.volume.min.free.space.percent",
       defaultValue = "-1",
       type = ConfigType.FLOAT,
       tags = { OZONE, CONTAINER, STORAGE, MANAGEMENT },
-      description = "" // not documented
+      description = "This determines the free space percent to be used for closing containers" +
+          " When the difference between volume capacity and used reaches (free.space.percent of volume capacity)," +
+          " containers that reside on this volume will be closed and no new containers" +
+          " would be allocated on this volume." +
+          " Either of min.free.space or min.free.space.percent should be configured, when both are set then" +
+          " min.free.space will be used."
   )
   private float minFreeSpaceRatio = MIN_FREE_SPACE_UNSET;
 
@@ -771,15 +785,19 @@ private void validateMinFreeSpace() {
     final boolean minFreeSpaceRatioConfigured = minFreeSpaceRatio >= 0;
 
     if (minFreeSpaceConfigured && minFreeSpaceRatioConfigured) {
-      LOG.warn("Only one of {}={} and {}={} should be set. With both set, default value ({}) will be used.",
+      // Only one property should be configured.
+      // Since both properties are configured, HDDS_DATANODE_VOLUME_MIN_FREE_SPACE is used to determine minFreeSpace
+      LOG.warn("Only one of {}={} and {}={} should be set. With both set, {} value will be used.",
           HDDS_DATANODE_VOLUME_MIN_FREE_SPACE,
           minFreeSpace,
           HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT,
           minFreeSpaceRatio,
-          HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_DEFAULT);
+          HDDS_DATANODE_VOLUME_MIN_FREE_SPACE);
+      minFreeSpaceRatio = MIN_FREE_SPACE_UNSET;
     }
 
-    if (minFreeSpaceConfigured == minFreeSpaceRatioConfigured) {
+    if (!minFreeSpaceConfigured && !minFreeSpaceRatioConfigured) {
+      // If both are not configured use defaultFreeSpace
       minFreeSpaceRatio = MIN_FREE_SPACE_UNSET;
       minFreeSpace = getDefaultFreeSpace();
     }

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/TestDatanodeConfiguration.java

@@ -31,7 +31,6 @@
 import static org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration.FAILED_VOLUMES_TOLERATED_DEFAULT;
 import static org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration.PERIODIC_DISK_CHECK_INTERVAL_MINUTES_DEFAULT;
 import static org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration.PERIODIC_DISK_CHECK_INTERVAL_MINUTES_KEY;
-import static org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration.getDefaultFreeSpace;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.util.concurrent.TimeUnit;
@@ -191,18 +190,19 @@ void rejectsInvalidMinFreeSpaceRatio() {
   }
 
   @Test
-  void usesDefaultFreeSpaceIfBothMinFreeSpacePropertiesSet() {
+  void useMinFreeSpaceIfBothMinFreeSpacePropertiesSet() {
     OzoneConfiguration conf = new OzoneConfiguration();
-    conf.setLong(DatanodeConfiguration.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE, 10000);
+    int minFreeSpace = 10000;
+    conf.setLong(DatanodeConfiguration.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE, minFreeSpace);
     conf.setFloat(DatanodeConfiguration.HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT, .5f);
 
     DatanodeConfiguration subject = conf.getObject(DatanodeConfiguration.class);
 
-    assertEquals(DatanodeConfiguration.getDefaultFreeSpace(), subject.getMinFreeSpace());
+    assertEquals(minFreeSpace, subject.getMinFreeSpace());
     assertEquals(DatanodeConfiguration.MIN_FREE_SPACE_UNSET, subject.getMinFreeSpaceRatio());
 
     for (long capacity : CAPACITIES) {
-      assertEquals(getDefaultFreeSpace(), subject.getMinFreeSpace(capacity));
+      assertEquals(minFreeSpace, subject.getMinFreeSpace(capacity));
     }
   }
 

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java

@@ -202,8 +202,8 @@ public void testMinFreeSpaceCalculator() throws Exception {
     assertEquals(minSpace, conf.getObject(DatanodeConfiguration.class).getMinFreeSpace(capacity));
 
     conf.setFloat(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT, 0.01f);
-    // default is 5GB
-    assertEquals(5L * 1024 * 1024 * 1024, conf.getObject(DatanodeConfiguration.class).getMinFreeSpace(capacity));
+    // When both are set, minSpace will be used
+    assertEquals(minSpace, conf.getObject(DatanodeConfiguration.class).getMinFreeSpace(capacity));
 
     // capacity * 1% = 10
     conf.unset(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE);