HDDS-241. Handle Volume in inconsistent state. Contributed by Hanisha Koneru.

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

@@ -42,6 +42,18 @@
  * HddsVolume represents volume in a datanode. {@link VolumeSet} maitains a
  * list of HddsVolumes, one for each volume in the Datanode.
  * {@link VolumeInfo} in encompassed by this class.
+ *
+ * The disk layout per volume is as follows:
+ * ../hdds/VERSION
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>>/metadata
+ * ../hdds/<<scmUuid>>/current/<<containerDir>>/<<containerID>>/<<dataDir>>
+ *
+ * Each hdds volume has its own VERSION file. The hdds volume will have one
+ * scmUuid directory for each SCM it is a part of (currently only one SCM is
+ * supported).
+ *
+ * During DN startup, if the VERSION file exists, we verify that the
+ * clusterID in the version file matches the clusterID from SCM.
  */
 public final class HddsVolume {
 
@@ -108,11 +120,6 @@ public HddsVolume build() throws IOException {
   }
 
   private HddsVolume(Builder b) throws IOException {
-    Preconditions.checkNotNull(b.volumeRootStr,
-        "Volume root dir cannot be null");
-    Preconditions.checkNotNull(b.datanodeUuid, "DatanodeUUID cannot be null");
-    Preconditions.checkNotNull(b.conf, "Configuration cannot be null");
-
     StorageLocation location = StorageLocation.parse(b.volumeRootStr);
     hddsRootDir = new File(location.getUri().getPath(), HDDS_VOLUME_DIR);
     this.state = VolumeState.NOT_INITIALIZED;
@@ -162,6 +169,10 @@ private void initialize() throws IOException {
       readVersionFile();
       setState(VolumeState.NORMAL);
       break;
+    case INCONSISTENT:
+      // Volume Root is in an inconsistent state. Skip loading this volume.
+      throw new IOException("Volume is in an " + VolumeState.INCONSISTENT +
+          " state. Skipped loading volume: " + hddsRootDir.getPath());
     default:
       throw new IOException("Unrecognized initial state : " +
           intialVolumeState + "of volume : " + hddsRootDir);
@@ -170,11 +181,23 @@ private void initialize() throws IOException {
 
   private VolumeState analyzeVolumeState() {
     if (!hddsRootDir.exists()) {
+      // Volume Root does not exist.
       return VolumeState.NON_EXISTENT;
     }
-    if (!getVersionFile().exists()) {
+    if (!hddsRootDir.isDirectory()) {
+      // Volume Root exists but is not a directory.
+      return VolumeState.INCONSISTENT;
+    }
+    File[] files = hddsRootDir.listFiles();
+    if (files == null || files.length == 0) {
+      // Volume Root exists and is empty.
       return VolumeState.NOT_FORMATTED;
     }
+    if (!getVersionFile().exists()) {
+      // Volume Root is non empty but VERSION file does not exist.
+      return VolumeState.INCONSISTENT;
+    }
+    // Volume Root and VERSION file exist.
     return VolumeState.NOT_INITIALIZED;
   }
 
@@ -321,11 +344,21 @@ public void shutdown() {
 
   /**
    * VolumeState represents the different states a HddsVolume can be in.
+   * NORMAL          => Volume can be used for storage
+   * FAILED          => Volume has failed due and can no longer be used for
+   *                    storing containers.
+   * NON_EXISTENT    => Volume Root dir does not exist
+   * INCONSISTENT    => Volume Root dir is not empty but VERSION file is
+   *                    missing or Volume Root dir is not a directory
+   * NOT_FORMATTED   => Volume Root exists but not formatted (no VERSION file)
+   * NOT_INITIALIZED => VERSION file exists but has not been verified for
+   *                    correctness.
    */
   public enum VolumeState {
     NORMAL,
     FAILED,
     NON_EXISTENT,
+    INCONSISTENT,
     NOT_FORMATTED,
     NOT_INITIALIZED
   }

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

@@ -202,18 +202,19 @@ private HddsVolume createVolume(String locationString,
 
 
   // Add a volume to VolumeSet
-  public void addVolume(String dataDir) throws IOException {
-    addVolume(dataDir, StorageType.DEFAULT);
+  public boolean addVolume(String dataDir) {
+    return addVolume(dataDir, StorageType.DEFAULT);
   }
 
   // Add a volume to VolumeSet
-  public void addVolume(String volumeRoot, StorageType storageType)
-      throws IOException {
+  public boolean addVolume(String volumeRoot, StorageType storageType) {
     String hddsRoot = HddsVolumeUtil.getHddsRoot(volumeRoot);
+    boolean success;
 
     try (AutoCloseableLock lock = volumeSetLock.acquire()) {
       if (volumeMap.containsKey(hddsRoot)) {
         LOG.warn("Volume : {} already exists in VolumeMap", hddsRoot);
+        success = false;
       } else {
         if (failedVolumeMap.containsKey(hddsRoot)) {
           failedVolumeMap.remove(hddsRoot);
@@ -225,8 +226,13 @@ public void addVolume(String volumeRoot, StorageType storageType)
 
         LOG.info("Added Volume : {} to VolumeSet",
             hddsVolume.getHddsRootDir().getPath());
+        success = true;
       }
+    } catch (IOException ex) {
+      LOG.error("Failed to add volume " + volumeRoot + " to VolumeSet", ex);
+      success = false;
     }
+    return success;
   }
 
   // Mark a volume as failed

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

@@ -18,22 +18,30 @@
 
 package org.apache.hadoop.ozone.container.common.volume;
 
+import java.io.IOException;
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.ozone.container.common.utils.HddsVolumeUtil;
 import org.apache.hadoop.test.GenericTestUtils.LogCapturer;
+
+import static org.apache.hadoop.ozone.container.common.volume.HddsVolume
+    .HDDS_VOLUME_DIR;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.Timeout;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
@@ -69,6 +77,28 @@ public void setup() throws Exception {
     initializeVolumeSet();
   }
 
+  @After
+  public void shutdown() throws IOException {
+    // Delete the hdds volume root dir
+    List<HddsVolume> volumes = new ArrayList<>();
+    volumes.addAll(volumeSet.getVolumesList());
+    volumes.addAll(volumeSet.getFailedVolumesList());
+
+    for (HddsVolume volume : volumes) {
+      FileUtils.deleteDirectory(volume.getHddsRootDir());
+    }
+  }
+
+  private boolean checkVolumeExistsInVolumeSet(String volume) {
+    for (HddsVolume hddsVolume : volumeSet.getVolumesList()) {
+      if (hddsVolume.getHddsRootDir().getPath().equals(
+          HddsVolumeUtil.getHddsRoot(volume))) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @Test
   public void testVolumeSetInitialization() throws Exception {
 
@@ -84,14 +114,18 @@ public void testVolumeSetInitialization() throws Exception {
   }
 
   @Test
-  public void testAddVolume() throws Exception {
+  public void testAddVolume() {
 
     assertEquals(2, volumeSet.getVolumesList().size());
 
     // Add a volume to VolumeSet
     String volume3 = baseDir + "disk3";
-    volumeSet.addVolume(volume3);
+//    File dir3 = new File(volume3, "hdds");
+//    File[] files = dir3.listFiles();
+//    System.out.println("------ " + files[0].getPath());
+    boolean success = volumeSet.addVolume(volume3);
 
+    assertTrue(success);
     assertEquals(3, volumeSet.getVolumesList().size());
     assertTrue("AddVolume did not add requested volume to VolumeSet",
         checkVolumeExistsInVolumeSet(volume3));
@@ -122,7 +156,6 @@ public void testFailVolume() throws Exception {
   @Test
   public void testRemoveVolume() throws Exception {
 
-    List<HddsVolume> volumesList = volumeSet.getVolumesList();
     assertEquals(2, volumeSet.getVolumesList().size());
 
     // Remove a volume from VolumeSet
@@ -141,13 +174,34 @@ public void testRemoveVolume() throws Exception {
         + expectedLogMessage, logs.getOutput().contains(expectedLogMessage));
   }
 
-  private boolean checkVolumeExistsInVolumeSet(String volume) {
-    for (HddsVolume hddsVolume : volumeSet.getVolumesList()) {
-      if (hddsVolume.getHddsRootDir().getPath().equals(
-          HddsVolumeUtil.getHddsRoot(volume))) {
-        return true;
-      }
-    }
-    return false;
+  @Test
+  public void testVolumeInInconsistentState() throws Exception {
+    assertEquals(2, volumeSet.getVolumesList().size());
+
+    // Add a volume to VolumeSet
+    String volume3 = baseDir + "disk3";
+
+    // Create the root volume dir and create a sub-directory within it.
+    File newVolume = new File(volume3, HDDS_VOLUME_DIR);
+    System.out.println("new volume root: " + newVolume);
+    newVolume.mkdirs();
+    assertTrue("Failed to create new volume root", newVolume.exists());
+    File dataDir = new File(newVolume, "chunks");
+    dataDir.mkdirs();
+    assertTrue(dataDir.exists());
+
+    // The new volume is in an inconsistent state as the root dir is
+    // non-empty but the version file does not exist. Add Volume should
+    // return false.
+    boolean success = volumeSet.addVolume(volume3);
+
+    assertFalse(success);
+    assertEquals(2, volumeSet.getVolumesList().size());
+    assertTrue("AddVolume should fail for an inconsistent volume",
+        !checkVolumeExistsInVolumeSet(volume3));
+
+    // Delete volume3
+    File volume = new File(volume3);
+    FileUtils.deleteDirectory(volume);
   }
 }

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java

@@ -25,6 +25,7 @@
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
 import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.container.common.impl.ContainerSet;
+import org.apache.hadoop.ozone.container.common.volume.HddsVolume;
 import org.apache.hadoop.ozone.container.common.volume.RoundRobinVolumeChoosingPolicy;
 import org.apache.hadoop.ozone.container.common.volume.VolumeSet;
 import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer;
@@ -60,21 +61,30 @@ public class TestOzoneContainer {
   public void setUp() throws Exception {
     conf = new OzoneConfiguration();
     conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_KEY, folder.getRoot()
-        .getAbsolutePath() + "," + folder.newFolder().getAbsolutePath());
+        .getAbsolutePath());
     conf.set(OzoneConfigKeys.OZONE_METADATA_DIRS, folder.newFolder().getAbsolutePath());
+  }
+
+  @Test
+  public void testBuildContainerMap() throws Exception {
     volumeSet = new VolumeSet(datanodeDetails.getUuidString(), conf);
     volumeChoosingPolicy = new RoundRobinVolumeChoosingPolicy();
 
+    // Format the volumes
+    for (HddsVolume volume : volumeSet.getVolumesList()) {
+      volume.format(UUID.randomUUID().toString());
+    }
+
+    // Add containers to disk
     for (int i=0; i<10; i++) {
       keyValueContainerData = new KeyValueContainerData(i, 1);
       keyValueContainer = new KeyValueContainer(
           keyValueContainerData, conf);
       keyValueContainer.create(volumeSet, volumeChoosingPolicy, scmId);
     }
-  }
 
-  @Test
-  public void testBuildContainerMap() throws Exception {
+    // When OzoneContainer is started, the containers from disk should be
+    // loaded into the containerSet.
     OzoneContainer ozoneContainer = new
         OzoneContainer(datanodeDetails, conf);
     ContainerSet containerset = ozoneContainer.getContainerSet();