Browse Source

HDDS-754. VolumeInfo#getScmUsed throws NPE.
Contributed by Hanisha Koneru.

(cherry picked from commit 773f0d1519715e3ddf77c139998cc12d7447da66)

Anu Engineer 6 years ago
parent
commit
382b73a109

+ 17 - 2
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java

@@ -95,15 +95,30 @@ public class VolumeInfo {
     this.usage = new VolumeUsage(root, b.conf);
     this.usage = new VolumeUsage(root, b.conf);
   }
   }
 
 
-  public long getCapacity() {
-    return configuredCapacity < 0 ? usage.getCapacity() : configuredCapacity;
+  public long getCapacity() throws IOException {
+    if (configuredCapacity < 0) {
+      if (usage == null) {
+        throw new IOException("Volume Usage thread is not running. This error" +
+            " is usually seen during DataNode shutdown.");
+      }
+      return usage.getCapacity();
+    }
+    return configuredCapacity;
   }
   }
 
 
   public long getAvailable() throws IOException {
   public long getAvailable() throws IOException {
+    if (usage == null) {
+      throw new IOException("Volume Usage thread is not running. This error " +
+          "is usually seen during DataNode shutdown.");
+    }
     return usage.getAvailable();
     return usage.getAvailable();
   }
   }
 
 
   public long getScmUsed() throws IOException {
   public long getScmUsed() throws IOException {
+    if (usage == null) {
+      throw new IOException("Volume Usage thread is not running. This error " +
+          "is usually seen during DataNode shutdown.");
+    }
     return usage.getScmUsed();
     return usage.getScmUsed();
   }
   }
 
 

+ 7 - 4
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeSet.java

@@ -372,18 +372,21 @@ public class VolumeSet {
       for (Map.Entry<String, HddsVolume> entry : volumeMap.entrySet()) {
       for (Map.Entry<String, HddsVolume> entry : volumeMap.entrySet()) {
         hddsVolume = entry.getValue();
         hddsVolume = entry.getValue();
         VolumeInfo volumeInfo = hddsVolume.getVolumeInfo();
         VolumeInfo volumeInfo = hddsVolume.getVolumeInfo();
-        long scmUsed = 0;
-        long remaining = 0;
+        long scmUsed;
+        long remaining;
+        long capacity;
         failed = false;
         failed = false;
         try {
         try {
           scmUsed = volumeInfo.getScmUsed();
           scmUsed = volumeInfo.getScmUsed();
           remaining = volumeInfo.getAvailable();
           remaining = volumeInfo.getAvailable();
+          capacity = volumeInfo.getCapacity();
         } catch (IOException ex) {
         } catch (IOException ex) {
           LOG.warn("Failed to get scmUsed and remaining for container " +
           LOG.warn("Failed to get scmUsed and remaining for container " +
-              "storage location {}", volumeInfo.getRootDir());
+              "storage location {}", volumeInfo.getRootDir(), ex);
           // reset scmUsed and remaining if df/du failed.
           // reset scmUsed and remaining if df/du failed.
           scmUsed = 0;
           scmUsed = 0;
           remaining = 0;
           remaining = 0;
+          capacity = 0;
           failed = true;
           failed = true;
         }
         }
 
 
@@ -392,7 +395,7 @@ public class VolumeSet {
         builder.setStorageLocation(volumeInfo.getRootDir())
         builder.setStorageLocation(volumeInfo.getRootDir())
             .setId(hddsVolume.getStorageID())
             .setId(hddsVolume.getStorageID())
             .setFailed(failed)
             .setFailed(failed)
-            .setCapacity(hddsVolume.getCapacity())
+            .setCapacity(capacity)
             .setRemaining(remaining)
             .setRemaining(remaining)
             .setScmUsed(scmUsed)
             .setScmUsed(scmUsed)
             .setStorageType(hddsVolume.getStorageType());
             .setStorageType(hddsVolume.getStorageType());

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

@@ -31,6 +31,7 @@ import org.junit.rules.TemporaryFolder;
 import org.mockito.Mockito;
 import org.mockito.Mockito;
 
 
 import java.io.File;
 import java.io.File;
+import java.io.IOException;
 import java.util.Properties;
 import java.util.Properties;
 import java.util.UUID;
 import java.util.UUID;
 
 
@@ -134,12 +135,14 @@ public class TestHddsVolume {
         scmUsedFile.exists());
         scmUsedFile.exists());
 
 
     try {
     try {
-      // Volume.getAvailable() should fail with NullPointerException as usage
-      // is shutdown.
+      // Volume.getAvailable() should fail with IOException
+      // as usage thread is shutdown.
       volume.getAvailable();
       volume.getAvailable();
       fail("HddsVolume#shutdown test failed");
       fail("HddsVolume#shutdown test failed");
     } catch (Exception ex){
     } catch (Exception ex){
-      assertTrue(ex instanceof NullPointerException);
+      assertTrue(ex instanceof IOException);
+      assertTrue(ex.getMessage().contains(
+          "Volume Usage thread is not running."));
     }
     }
   }
   }
 }
 }

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

@@ -222,8 +222,10 @@ public class TestVolumeSet {
         // getAvailable() should throw null pointer exception as usage is null.
         // getAvailable() should throw null pointer exception as usage is null.
         volume.getAvailable();
         volume.getAvailable();
         fail("Volume shutdown failed.");
         fail("Volume shutdown failed.");
-      } catch (NullPointerException ex) {
+      } catch (IOException ex) {
         // Do Nothing. Exception is expected.
         // Do Nothing. Exception is expected.
+        assertTrue(ex.getMessage().contains(
+            "Volume Usage thread is not running."));
       }
       }
     }
     }
   }
   }

+ 2 - 1
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestNodeFailure.java

@@ -99,7 +99,7 @@ public class TestNodeFailure {
     }
     }
   }
   }
 
 
-  @Test
+  @Test(timeout = 300_000L)
   public void testPipelineFail() throws InterruptedException, IOException,
   public void testPipelineFail() throws InterruptedException, IOException,
       TimeoutException {
       TimeoutException {
     Assert.assertEquals(ratisContainer1.getPipeline().getLifeCycleState(),
     Assert.assertEquals(ratisContainer1.getPipeline().getLifeCycleState(),
@@ -118,6 +118,7 @@ public class TestNodeFailure {
     Assert.assertNull(containerManager.getPipelineSelector()
     Assert.assertNull(containerManager.getPipelineSelector()
         .getPipeline(pipelineToFail.getId()));
         .getPipeline(pipelineToFail.getId()));
     // Now restart the datanode and make sure that a new pipeline is created.
     // Now restart the datanode and make sure that a new pipeline is created.
+    cluster.setWaitForClusterToBeReadyTimeout(300000);
     cluster.restartHddsDatanode(dnToFail, true);
     cluster.restartHddsDatanode(dnToFail, true);
     ContainerWithPipeline ratisContainer3 =
     ContainerWithPipeline ratisContainer3 =
         containerManager.allocateContainer(RATIS, THREE, "testOwner");
         containerManager.allocateContainer(RATIS, THREE, "testOwner");

+ 8 - 0
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneCluster.java

@@ -66,6 +66,14 @@ public interface MiniOzoneCluster {
    */
    */
   void waitForClusterToBeReady() throws TimeoutException, InterruptedException;
   void waitForClusterToBeReady() throws TimeoutException, InterruptedException;
 
 
+  /**
+   * Sets the timeout value after which
+   * {@link MiniOzoneCluster#waitForClusterToBeReady} times out.
+   *
+   * @param timeoutInMs timeout value in milliseconds
+   */
+  void setWaitForClusterToBeReadyTimeout(int timeoutInMs);
+
   /**
   /**
    * Waits/blocks till the cluster is out of chill mode.
    * Waits/blocks till the cluster is out of chill mode.
    *
    *

+ 15 - 1
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java

@@ -90,6 +90,9 @@ public final class MiniOzoneClusterImpl implements MiniOzoneCluster {
   private final OzoneManager ozoneManager;
   private final OzoneManager ozoneManager;
   private final List<HddsDatanodeService> hddsDatanodes;
   private final List<HddsDatanodeService> hddsDatanodes;
 
 
+  // Timeout for the cluster to be ready
+  private int waitForClusterToBeReadyTimeout = 60000; // 1 min
+
   /**
   /**
    * Creates a new MiniOzoneCluster.
    * Creates a new MiniOzoneCluster.
    *
    *
@@ -122,7 +125,18 @@ public final class MiniOzoneClusterImpl implements MiniOzoneCluster {
           isReady? "Cluster is ready" : "Waiting for cluster to be ready",
           isReady? "Cluster is ready" : "Waiting for cluster to be ready",
           healthy, hddsDatanodes.size());
           healthy, hddsDatanodes.size());
       return isReady;
       return isReady;
-    }, 1000, 60 * 1000); //wait for 1 min.
+    }, 1000, waitForClusterToBeReadyTimeout);
+  }
+
+  /**
+   * Sets the timeout value after which
+   * {@link MiniOzoneClusterImpl#waitForClusterToBeReady} times out.
+   *
+   * @param timeoutInMs timeout value in milliseconds
+   */
+  @Override
+  public void setWaitForClusterToBeReadyTimeout(int timeoutInMs) {
+    waitForClusterToBeReadyTimeout = timeoutInMs;
   }
   }
 
 
   /**
   /**