From 5eb24ef7e7b8fb61a5f5b88bae3596b30aaeb60b Mon Sep 17 00:00:00 2001 From: Uma Maheswara Rao G Date: Wed, 12 Jul 2017 17:56:56 -0700 Subject: [PATCH] HDFS-11264: [SPS]: Double checks to ensure that SPS/Mover are not running together. Contributed by Rakesh R. --- .../namenode/StoragePolicySatisfier.java | 53 ++++++++++--------- .../namenode/TestStoragePolicySatisfier.java | 3 +- ...StoragePolicySatisfierWithStripedFile.java | 5 +- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/StoragePolicySatisfier.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/StoragePolicySatisfier.java index 97cbf1bbe3f..00b4cd0260f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/StoragePolicySatisfier.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/StoragePolicySatisfier.java @@ -128,6 +128,14 @@ public class StoragePolicySatisfier implements Runnable { */ public synchronized void start(boolean reconfigStart) { isRunning = true; + if (checkIfMoverRunning()) { + isRunning = false; + LOG.error( + "Stopping StoragePolicySatisfier thread " + "as Mover ID file " + + HdfsServerConstants.MOVER_ID_PATH.toString() + + " been opened. Maybe a Mover instance is running!"); + return; + } if (reconfigStart) { LOG.info("Starting StoragePolicySatisfier, as admin requested to " + "activate it."); @@ -211,20 +219,6 @@ public class StoragePolicySatisfier implements Runnable { @Override public void run() { - boolean isMoverRunning = !checkIfMoverRunning(); - synchronized (this) { - isRunning = isMoverRunning; - if (!isRunning) { - // Stopping monitor thread and clearing queues as well - this.clearQueues(); - this.storageMovementsMonitor.stopGracefully(); - LOG.error( - "Stopping StoragePolicySatisfier thread " + "as Mover ID file " - + HdfsServerConstants.MOVER_ID_PATH.toString() - + " been opened. Maybe a Mover instance is running!"); - return; - } - } while (namesystem.isRunning() && isRunning) { try { if (!namesystem.isInSafeMode()) { @@ -274,25 +268,34 @@ public class StoragePolicySatisfier implements Runnable { // we want to check block movements. Thread.sleep(3000); } catch (Throwable t) { - synchronized (this) { + handleException(t); + } + } + } + + private void handleException(Throwable t) { + // double check to avoid entering into synchronized block. + if (isRunning) { + synchronized (this) { + if (isRunning) { isRunning = false; // Stopping monitor thread and clearing queues as well this.clearQueues(); this.storageMovementsMonitor.stopGracefully(); - } - if (!namesystem.isRunning()) { - LOG.info("Stopping StoragePolicySatisfier."); - if (!(t instanceof InterruptedException)) { - LOG.info("StoragePolicySatisfier received an exception" - + " while shutting down.", t); + if (!namesystem.isRunning()) { + LOG.info("Stopping StoragePolicySatisfier."); + if (!(t instanceof InterruptedException)) { + LOG.info("StoragePolicySatisfier received an exception" + + " while shutting down.", t); + } + return; } - break; } - LOG.error("StoragePolicySatisfier thread received runtime exception. " - + "Stopping Storage policy satisfier work", t); - break; } } + LOG.error("StoragePolicySatisfier thread received runtime exception. " + + "Stopping Storage policy satisfier work", t); + return; } private BlocksMovingAnalysisStatus analyseBlocksStorageMovementsAndAssignToDN( diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfier.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfier.java index 71278956fe1..be7236b0813 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfier.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfier.java @@ -927,7 +927,8 @@ public class TestStoragePolicySatisfier { String fooDir = "/foo"; client.mkdirs(fooDir, new FsPermission((short) 777), true); // set an EC policy on "/foo" directory - client.setErasureCodingPolicy(fooDir, null); + client.setErasureCodingPolicy(fooDir, + StripedFileTestUtil.getDefaultECPolicy().getName()); // write file to fooDir final String testFile = "/foo/bar"; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfierWithStripedFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfierWithStripedFile.java index 195c9e3ca51..f905eadc9a5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfierWithStripedFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestStoragePolicySatisfierWithStripedFile.java @@ -323,6 +323,8 @@ public class TestStoragePolicySatisfierWithStripedFile { conf.set(DFSConfigKeys .DFS_STORAGE_POLICY_SATISFIER_RECHECK_TIMEOUT_MILLIS_KEY, "3000"); + conf.set(DFSConfigKeys.DFS_NAMENODE_EC_POLICIES_ENABLED_KEY, + StripedFileTestUtil.getDefaultECPolicy().getName()); initConfWithStripe(conf, defaultStripeBlockSize); final MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf) .numDataNodes(numOfDatanodes) @@ -346,7 +348,8 @@ public class TestStoragePolicySatisfierWithStripedFile { Path barDir = new Path("/bar"); fs.mkdirs(barDir); // set an EC policy on "/bar" directory - fs.setErasureCodingPolicy(barDir, null); + fs.setErasureCodingPolicy(barDir, + StripedFileTestUtil.getDefaultECPolicy().getName()); // write file to barDir final Path fooFile = new Path("/bar/foo");