HDDS-10505. Move space reservation logic to VolumeUsage (apache#6370)

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

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

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

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

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

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<String> 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;
+  }
+
 }