HDDS-257. Hook up VolumeSet#shutdown from HddsDispatcher#shutdown. Contributed by Hanisha Koneru
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/ba25d27d Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/ba25d27d Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/ba25d27d Branch: refs/heads/HADOOP-15461 Commit: ba25d27ddb8d32abf5e1314a51eec7cad789b316 Parents: de894d3 Author: Bharat Viswanadham <bha...@apache.org> Authored: Fri Jul 20 12:41:52 2018 -0700 Committer: Bharat Viswanadham <bha...@apache.org> Committed: Fri Jul 20 12:41:52 2018 -0700 ---------------------------------------------------------------------- .../container/common/impl/HddsDispatcher.java | 2 ++ .../container/common/volume/HddsVolume.java | 2 -- .../container/common/volume/VolumeInfo.java | 8 ++++++ .../container/common/volume/VolumeSet.java | 18 +++++++++++++- .../container/common/volume/VolumeUsage.java | 17 ------------- .../container/common/volume/TestVolumeSet.java | 26 +++++++++++++++++--- 6 files changed, 50 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index f0c2aa9..bee8417 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -81,6 +81,8 @@ public class HddsDispatcher implements ContainerDispatcher { @Override public void shutdown() { + // Shutdown the volumes + volumeSet.shutdown(); } @Override http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java ---------------------------------------------------------------------- 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 6468720..0cbfd9f 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 @@ -370,6 +370,4 @@ public final class HddsVolume { public void setScmUsageForTesting(GetSpaceUsed scmUsageForTest) { volumeInfo.setScmUsageForTesting(scmUsageForTest); } - - } http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java index 4b13d45..62fca63 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfo.java @@ -129,4 +129,12 @@ public class VolumeInfo { public void setScmUsageForTesting(GetSpaceUsed scmUsageForTest) { usage.setScmUsageForTesting(scmUsageForTest); } + + /** + * Only for testing. Do not use otherwise. + */ + @VisibleForTesting + public VolumeUsage getUsageForTesting() { + return usage; + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeSet.java ---------------------------------------------------------------------- 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 2dd4763..4dfde37 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 @@ -23,9 +23,11 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.StorageType; -import org.apache.hadoop.hdds.protocol.DatanodeDetails; + import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_DATANODE_DATA_DIR_KEY; +import static org.apache.hadoop.util.RunJar.SHUTDOWN_HOOK_PRIORITY; + import org.apache.hadoop.hdfs.server.datanode.StorageLocation; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos; @@ -40,6 +42,7 @@ import org.apache.hadoop.ozone.container.common.interfaces.VolumeChoosingPolicy; import org.apache.hadoop.util.AutoCloseableLock; import org.apache.hadoop.util.DiskChecker.DiskOutOfSpaceException; import org.apache.hadoop.util.InstrumentedLock; +import org.apache.hadoop.util.ShutdownHookManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -89,6 +92,8 @@ public class VolumeSet { private final String datanodeUuid; private String clusterID; + private Runnable shutdownHook; + public VolumeSet(String dnUuid, Configuration conf) throws DiskOutOfSpaceException { this(dnUuid, null, conf); @@ -155,6 +160,13 @@ public class VolumeSet { if (volumeMap.size() == 0) { throw new DiskOutOfSpaceException("No storage location configured"); } + + // Ensure volume threads are stopped and scm df is saved during shutdown. + shutdownHook = () -> { + shutdown(); + }; + ShutdownHookManager.get().addShutdownHook(shutdownHook, + SHUTDOWN_HOOK_PRIORITY); } /** @@ -296,6 +308,10 @@ public class VolumeSet { ex); } } + + if (shutdownHook != null) { + ShutdownHookManager.get().removeShutdownHook(shutdownHook); + } } @VisibleForTesting http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java ---------------------------------------------------------------------- diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java index e10d1d4..2c7563e 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java @@ -24,8 +24,6 @@ import org.apache.hadoop.fs.CachingGetSpaceUsed; import org.apache.hadoop.fs.DF; import org.apache.hadoop.fs.GetSpaceUsed; import org.apache.hadoop.io.IOUtils; -import static org.apache.hadoop.util.RunJar.SHUTDOWN_HOOK_PRIORITY; -import org.apache.hadoop.util.ShutdownHookManager; import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,7 +47,6 @@ public class VolumeUsage { private final DF df; private final File scmUsedFile; private GetSpaceUsed scmUsage; - private Runnable shutdownHook; private static final String DU_CACHE_FILE = "scmUsed"; private volatile boolean scmUsedSaved = false; @@ -72,15 +69,6 @@ public class VolumeUsage { .setConf(conf) .setInitialUsed(loadScmUsed()) .build(); - - // Ensure scm df is saved during shutdown. - shutdownHook = () -> { - if (!scmUsedSaved) { - saveScmUsed(); - } - }; - ShutdownHookManager.get().addShutdownHook(shutdownHook, - SHUTDOWN_HOOK_PRIORITY); } long getCapacity() { @@ -106,11 +94,6 @@ public class VolumeUsage { public void shutdown() { saveScmUsed(); - scmUsedSaved = true; - - if (shutdownHook != null) { - ShutdownHookManager.get().removeShutdownHook(shutdownHook); - } if (scmUsage instanceof CachingGetSpaceUsed) { IOUtils.cleanupWithLogger(null, ((CachingGetSpaceUsed) scmUsage)); http://git-wip-us.apache.org/repos/asf/hadoop/blob/ba25d27d/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java ---------------------------------------------------------------------- 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 4f75b9a..3ee9343 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 @@ -34,8 +34,10 @@ import static org.apache.hadoop.ozone.container.common.volume.HddsVolume import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -87,6 +89,7 @@ public class TestVolumeSet { for (HddsVolume volume : volumes) { FileUtils.deleteDirectory(volume.getHddsRootDir()); } + volumeSet.shutdown(); } private boolean checkVolumeExistsInVolumeSet(String volume) { @@ -120,9 +123,6 @@ public class TestVolumeSet { // Add a volume to VolumeSet String volume3 = baseDir + "disk3"; -// File dir3 = new File(volume3, "hdds"); -// File[] files = dir3.listFiles(); -// System.out.println("------ " + files[0].getPath()); boolean success = volumeSet.addVolume(volume3); assertTrue(success); @@ -204,4 +204,24 @@ public class TestVolumeSet { File volume = new File(volume3); FileUtils.deleteDirectory(volume); } + + @Test + public void testShutdown() throws Exception { + List<HddsVolume> volumesList = volumeSet.getVolumesList(); + + volumeSet.shutdown(); + + // Verify that the volumes are shutdown and the volumeUsage is set to null. + for (HddsVolume volume : volumesList) { + Assert.assertNull(volume.getVolumeInfo().getUsageForTesting()); + try { + // getAvailable() should throw null pointer exception as usage is null. + volume.getAvailable(); + fail("Volume shutdown failed."); + } catch (NullPointerException ex) { + // Do Nothing. Exception is expected. + } + } + + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org