HDDS-8054. Fix NPE in metrics for failed volume (apache#4340)

(cherry picked from commit c144a7d)

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/DbVolume.java

@@ -56,9 +56,11 @@ protected DbVolume(Builder b) throws IOException {
     super(b);
 
     this.hddsDbStorePathMap = new HashMap<>();
-    if (!b.getFailedVolume()) {
+    if (!b.getFailedVolume() && getVolumeInfo().isPresent()) {
       LOG.info("Creating DbVolume: {} of storage type : {} capacity : {}",
-          getStorageDir(), b.getStorageType(), getVolumeInfo().getCapacity());
+              getStorageDir(), b.getStorageType(),
+              getVolumeInfo().get().getCapacity());
+
       initialize();
     }
   }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java

@@ -106,7 +106,7 @@ public HddsVolume build() throws IOException {
   private HddsVolume(Builder b) throws IOException {
     super(b);
 
-    if (!b.getFailedVolume()) {
+    if (!b.getFailedVolume() && getVolumeInfo().isPresent()) {
       this.setState(VolumeState.NOT_INITIALIZED);
       this.volumeIOStats = new VolumeIOStats(b.getVolumeRootStr(),
           this.getStorageDir().toString());
@@ -115,7 +115,8 @@ private HddsVolume(Builder b) throws IOException {
       this.committedBytes = new AtomicLong(0);
 
       LOG.info("Creating HddsVolume: {} of storage type : {} capacity : {}",
-          getStorageDir(), b.getStorageType(), getVolumeInfo().getCapacity());
+          getStorageDir(), b.getStorageType(),
+              getVolumeInfo().get().getCapacity());
 
       initialize();
     } else {

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/MutableVolumeSet.java

@@ -25,6 +25,7 @@
 import java.util.EnumMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -485,28 +486,32 @@ public StorageLocationReport[] getStorageReport() {
       StorageVolume volume;
       for (Map.Entry<String, StorageVolume> entry : volumeMap.entrySet()) {
         volume = entry.getValue();
-        VolumeInfo volumeInfo = volume.getVolumeInfo();
-        long scmUsed;
-        long remaining;
-        long capacity;
-        failed = false;
-        try {
-          scmUsed = volumeInfo.getScmUsed();
-          remaining = volumeInfo.getAvailable();
-          capacity = volumeInfo.getCapacity();
-        } catch (UncheckedIOException ex) {
-          LOG.warn("Failed to get scmUsed and remaining for container " +
-              "storage location {}", volumeInfo.getRootDir(), ex);
-          // reset scmUsed and remaining if df/du failed.
-          scmUsed = 0;
-          remaining = 0;
-          capacity = 0;
-          failed = true;
+        Optional<VolumeInfo> volumeInfo = volume.getVolumeInfo();
+        long scmUsed = 0;
+        long remaining = 0;
+        long capacity = 0;
+        String rootDir = "";
+        failed = true;
+        if (volumeInfo.isPresent()) {
+          try {
+            rootDir = volumeInfo.get().getRootDir();
+            scmUsed = volumeInfo.get().getScmUsed();
+            remaining = volumeInfo.get().getAvailable();
+            capacity = volumeInfo.get().getCapacity();
+            failed = false;
+          } catch (UncheckedIOException ex) {
+            LOG.warn("Failed to get scmUsed and remaining for container " +
+                    "storage location {}", volumeInfo.get().getRootDir(), ex);
+            // reset scmUsed and remaining if df/du failed.
+            scmUsed = 0;
+            remaining = 0;
+            capacity = 0;
+          }
         }
 
         StorageLocationReport.Builder builder =
             StorageLocationReport.newBuilder();
-        builder.setStorageLocation(volumeInfo.getRootDir())
+        builder.setStorageLocation(rootDir)
             .setId(volume.getStorageID())
             .setFailed(failed)
             .setCapacity(capacity)

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java

@@ -37,6 +37,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.UUID;
 
@@ -105,7 +106,7 @@ public enum VolumeState {
 
   private final File storageDir;
 
-  private final VolumeInfo volumeInfo;
+  private final Optional<VolumeInfo> volumeInfo;
 
   private final VolumeSet volumeSet;
 
@@ -115,18 +116,19 @@ protected StorageVolume(Builder<?> b) throws IOException {
     if (!b.failedVolume) {
       StorageLocation location = StorageLocation.parse(b.volumeRootStr);
       storageDir = new File(location.getUri().getPath(), b.storageDirStr);
-      this.volumeInfo = new VolumeInfo.Builder(b.volumeRootStr, b.conf)
+      this.volumeInfo = Optional.of(
+              new VolumeInfo.Builder(b.volumeRootStr, b.conf)
           .storageType(b.storageType)
           .usageCheckFactory(b.usageCheckFactory)
-          .build();
+          .build());
       this.volumeSet = b.volumeSet;
       this.state = VolumeState.NOT_INITIALIZED;
       this.clusterID = b.clusterID;
       this.datanodeUuid = b.datanodeUuid;
       this.conf = b.conf;
     } else {
       storageDir = new File(b.volumeRootStr);
-      this.volumeInfo = null;
+      this.volumeInfo = Optional.empty();
       this.volumeSet = null;
       this.storageID = UUID.randomUUID().toString();
       this.state = VolumeState.FAILED;
@@ -368,16 +370,22 @@ public StorageType getStorageType() {
     }
   }
 
+  public String getVolumeRootDir() {
+    return volumeInfo.map(VolumeInfo::getRootDir).orElse(null);
+  }
+
   public long getCapacity() {
-    return volumeInfo != null ? volumeInfo.getCapacity() : 0;
+    return volumeInfo.map(VolumeInfo::getCapacity).orElse(0L);
   }
 
   public long getAvailable() {
-    return volumeInfo != null ? volumeInfo.getAvailable() : 0;
+    return volumeInfo.map(VolumeInfo::getAvailable).orElse(0L);
+
   }
 
   public long getUsedSpace() {
-    return volumeInfo != null ? volumeInfo.getScmUsed() : 0;
+    return volumeInfo.map(VolumeInfo::getScmUsed).orElse(0L);
+
   }
 
   public File getStorageDir() {
@@ -389,34 +397,30 @@ public String getWorkingDir() {
   }
 
   public void refreshVolumeInfo() {
-    volumeInfo.refreshNow();
+    volumeInfo.ifPresent(VolumeInfo::refreshNow);
   }
 
-  public VolumeInfo getVolumeInfo() {
+  public Optional<VolumeInfo> getVolumeInfo() {
     return this.volumeInfo;
   }
 
   public void incrementUsedSpace(long usedSpace) {
-    if (this.volumeInfo != null) {
-      this.volumeInfo.incrementUsedSpace(usedSpace);
-    }
+    volumeInfo.ifPresent(volInfo -> volInfo
+            .incrementUsedSpace(usedSpace));
   }
 
   public void decrementUsedSpace(long reclaimedSpace) {
-    if (this.volumeInfo != null) {
-      this.volumeInfo.decrementUsedSpace(reclaimedSpace);
-    }
+    volumeInfo.ifPresent(volInfo -> volInfo
+            .decrementUsedSpace(reclaimedSpace));
   }
 
   public VolumeSet getVolumeSet() {
     return this.volumeSet;
   }
 
   public StorageType getStorageType() {
-    if (this.volumeInfo != null) {
-      return this.volumeInfo.getStorageType();
-    }
-    return StorageType.DEFAULT;
+    return volumeInfo.map(VolumeInfo::getStorageType)
+            .orElse(StorageType.DEFAULT);
   }
 
   public String getStorageID() {
@@ -457,16 +461,12 @@ public ConfigurationSource getConf() {
 
   public void failVolume() {
     setState(VolumeState.FAILED);
-    if (volumeInfo != null) {
-      volumeInfo.shutdownUsageThread();
-    }
+    volumeInfo.ifPresent(VolumeInfo::shutdownUsageThread);
   }
 
   public void shutdown() {
     setState(VolumeState.NON_EXISTENT);
-    if (volumeInfo != null) {
-      volumeInfo.shutdownUsageThread();
-    }
+    volumeInfo.ifPresent(VolumeInfo::shutdownUsageThread);
   }
 
   /**

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfoMetrics.java

@@ -104,23 +104,26 @@ public String getMetricsSourceName() {
    */
   @Metric("Returns the Used space")
   public long getUsed() {
-    return volume.getVolumeInfo().getScmUsed();
+    return volume.getVolumeInfo().map(VolumeInfo::getScmUsed)
+            .orElse(0L);
   }
 
   /**
    * Return the Total Available capacity of the Volume.
    */
   @Metric("Returns the Available space")
   public long getAvailable() {
-    return volume.getVolumeInfo().getAvailable();
+    return volume.getVolumeInfo().map(VolumeInfo::getAvailable)
+            .orElse(0L);
   }
 
   /**
    * Return the Total Reserved of the Volume.
    */
   @Metric("Fetches the Reserved Space")
   public long getReserved() {
-    return volume.getVolumeInfo().getReservedInBytes();
+    return volume.getVolumeInfo().map(VolumeInfo::getReservedInBytes)
+            .orElse(0L);
   }
 
   /**

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

@@ -369,6 +369,25 @@ public void testDbStoreClosedOnBadVolumeWithDbVolumes() throws IOException {
     assertEquals(0, DatanodeStoreCache.getInstance().size());
   }
 
+  @Test
+  public void testFailedVolumeSpace() throws IOException {
+    // Build failed volume
+    HddsVolume volume = volumeBuilder.failedVolume(true).build();
+    VolumeInfoMetrics volumeInfoMetrics = volume.getVolumeInfoStats();
+
+    try {
+      // In case of failed volume all stats should return 0.
+      assertEquals(0, volumeInfoMetrics.getUsed());
+      assertEquals(0, volumeInfoMetrics.getAvailable());
+      assertEquals(0, volumeInfoMetrics.getCapacity());
+      assertEquals(0, volumeInfoMetrics.getReserved());
+      assertEquals(0, volumeInfoMetrics.getTotalCapacity());
+    } finally {
+      // Shutdown the volume.
+      volume.shutdown();
+    }
+  }
+
   private MutableVolumeSet createDbVolumeSet() throws IOException {
     File dbVolumeDir = folder.newFolder();
     CONF.set(OzoneConfigKeys.HDDS_DATANODE_CONTAINER_DB_DIR,

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

@@ -66,13 +66,15 @@ public void testVolumeCapacityAfterReserve() throws Exception {
 
     long volumeCapacity = hddsVolume.getCapacity();
     //Gets the actual total capacity
-    long totalCapacity = hddsVolume.getVolumeInfo()
+    long totalCapacity = hddsVolume.getVolumeInfo().get()
         .getUsageForTesting().getCapacity();
-    long reservedCapacity = hddsVolume.getVolumeInfo().getReservedInBytes();
+    long reservedCapacity = hddsVolume.getVolumeInfo().get()
+            .getReservedInBytes();
     //Volume Capacity with Reserved
     long volumeCapacityReserved = totalCapacity - reservedCapacity;
 
-    long reservedFromVolume = hddsVolume.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume = hddsVolume.getVolumeInfo().get()
+            .getReservedInBytes();
     long reservedCalculated = (long) Math.ceil(totalCapacity * percentage);
 
     Assert.assertEquals(reservedFromVolume, reservedCalculated);
@@ -92,7 +94,8 @@ public void testReservedWhenBothConfigSet() throws Exception {
         folder.getRoot() + ":500B");
     HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
 
-    long reservedFromVolume = hddsVolume.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume = hddsVolume.getVolumeInfo().get()
+            .getReservedInBytes();
     Assert.assertEquals(reservedFromVolume, 500);
   }
 
@@ -101,7 +104,8 @@ public void testReservedToZeroWhenBothConfigNotSet() throws Exception {
     OzoneConfiguration conf = new OzoneConfiguration();
     HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
 
-    long reservedFromVolume = hddsVolume.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume = hddsVolume.getVolumeInfo().get()
+            .getReservedInBytes();
     Assert.assertEquals(reservedFromVolume, 0);
   }
 
@@ -114,10 +118,11 @@ public void testFallbackToPercentConfig() throws Exception {
         temp.getRoot() + ":500B");
     HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
 
-    long reservedFromVolume = hddsVolume.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume = hddsVolume.getVolumeInfo().get()
+            .getReservedInBytes();
     Assert.assertNotEquals(reservedFromVolume, 0);
 
-    long totalCapacity = hddsVolume.getVolumeInfo()
+    long totalCapacity = hddsVolume.getVolumeInfo().get()
         .getUsageForTesting().getCapacity();
     float percentage = conf.getFloat(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT,
         HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT);
@@ -134,7 +139,8 @@ public void testInvalidConfig() throws Exception {
         folder.getRoot() + ":500C");
     HddsVolume hddsVolume1 = volumeBuilder.conf(conf1).build();
 
-    long reservedFromVolume1 = hddsVolume1.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume1 = hddsVolume1.getVolumeInfo().get()
+            .getReservedInBytes();
     Assert.assertEquals(reservedFromVolume1, 0);
 
     OzoneConfiguration conf2 = new OzoneConfiguration();
@@ -143,7 +149,8 @@ public void testInvalidConfig() throws Exception {
     conf2.set(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, "20");
     HddsVolume hddsVolume2 = volumeBuilder.conf(conf2).build();
 
-    long reservedFromVolume2 = hddsVolume2.getVolumeInfo().getReservedInBytes();
+    long reservedFromVolume2 = hddsVolume2.getVolumeInfo().get()
+            .getReservedInBytes();
     Assert.assertEquals(reservedFromVolume2, 0);
   }
 }

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

@@ -219,7 +219,8 @@ public void testShutdown() throws Exception {
 
     // Verify that volume usage can be queried during shutdown.
     for (StorageVolume volume : volumesList) {
-      Assert.assertNotNull(volume.getVolumeInfo().getUsageForTesting());
+      Assert.assertNotNull(volume.getVolumeInfo().get()
+              .getUsageForTesting());
       volume.getAvailable();
     }
   }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java

@@ -385,7 +385,7 @@ public void testMultipleDataDirs() throws Exception {
 
     volumeList.forEach(storageVolume -> Assert.assertEquals(
             (long) StorageSize.parse(reservedSpace).getValue(),
-            storageVolume.getVolumeInfo().getReservedInBytes()));
+            storageVolume.getVolumeInfo().get().getReservedInBytes()));
   }
 
   private static void assertDetailsEquals(DatanodeDetails expected,