From d5d444732bf5c3f3cfc681f8d87e0681a7471f2f Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Wed, 18 Jul 2018 09:38:43 -0700 Subject: [PATCH] HDDS-241. Handle Volume in inconsistent state. Contributed by Hanisha Koneru. --- .../container/common/volume/HddsVolume.java | 45 +++++++++-- .../container/common/volume/VolumeSet.java | 14 +++- .../common/volume/TestVolumeSet.java | 78 ++++++++++++++++--- .../ozoneimpl/TestOzoneContainer.java | 18 ++++- 4 files changed, 129 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java index 1e71494446..6468720dc3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java +++ b/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/<>/current/<>/<>/metadata + * ../hdds/<>/current/<>/<>/<> + * + * 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 } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeSet.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeSet.java index 692a9d1f56..2dd476300f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeSet.java +++ b/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 diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java index 41f75bdf56..4f75b9a358 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java +++ b/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 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 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); } } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java index 27c6528065..284ffa386e 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainer.java +++ b/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();