HDFS-2577. NN fails to start since it tries to start secret manager in safemode. Contributed by Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-1623@1205689 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-1623.txt

@@ -21,3 +21,5 @@ HDFS-2418. Change ConfiguredFailoverProxyProvider to take advantage of HDFS-2231
 HDFS-2393. Mark appropriate methods of ClientProtocol with the idempotent annotation. (atm)
 
 HDFS-2523. Small NN fixes to include HAServiceProtocol and prevent NPE on shutdown. (todd)
+
+HDFS-2577. NN fails to start since it tries to start secret manager in safemode. (todd)

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -258,7 +258,6 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
 
   LeaseManager leaseManager = new LeaseManager(this); 
 
-  Daemon lmthread = null;   // LeaseMonitor thread
   Daemon smmthread = null;  // SafeModeMonitor thread
   
   Daemon nnrmthread = null; // NamenodeResourceMonitor thread
@@ -450,9 +449,10 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     LOG.info("Starting services required for active state");
     writeLock();
     try {
-      startSecretManager();
+      if (UserGroupInformation.isSecurityEnabled()) {
-      lmthread = new Daemon(leaseManager.new Monitor());
+        startSecretManager();
-      lmthread.start();
+      }
+      leaseManager.startMonitor();
     } finally {
       writeUnlock();
     }
@@ -467,14 +467,8 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     writeLock();
     try {
       stopSecretManager();
-      if (lmthread != null) {
+      if (leaseManager != null) {
-        try {
+        leaseManager.stopMonitor();
-          lmthread.interrupt();
-          lmthread.join(3000);
-        } catch (InterruptedException ie) {
-          LOG.warn("Encountered exception ", ie);
-        }
-        lmthread = null;
       }
     } finally {
       writeUnlock();
@@ -542,6 +536,10 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     this.fsLock.writeLock().lock();
   }
   @Override
+  public void writeLockInterruptibly() throws InterruptedException {
+    this.fsLock.writeLock().lockInterruptibly();
+  }
+  @Override
   public void writeUnlock() {
     this.fsLock.writeLock().unlock();
   }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/LeaseManager.java

@@ -34,6 +34,10 @@ import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.UnresolvedLinkException;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants;
+import org.apache.hadoop.util.Daemon;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 
 import static org.apache.hadoop.hdfs.server.common.Util.now;
 
@@ -82,6 +86,9 @@ public class LeaseManager {
   //
   private SortedMap<String, Lease> sortedLeasesByPath = new TreeMap<String, Lease>();
 
+  private Daemon lmthread;
+  private volatile boolean shouldRunMonitor;
+
   LeaseManager(FSNamesystem fsnamesystem) {this.fsnamesystem = fsnamesystem;}
 
   Lease getLease(String holder) {
@@ -367,18 +374,18 @@ public class LeaseManager {
 
     /** Check leases periodically. */
     public void run() {
-      for(; fsnamesystem.isRunning(); ) {
+      for(; shouldRunMonitor && fsnamesystem.isRunning(); ) {
-        fsnamesystem.writeLock();
         try {
-          if (!fsnamesystem.isInSafeMode()) {
+          fsnamesystem.writeLockInterruptibly();
-            checkLeases();
+          try {
+            if (!fsnamesystem.isInSafeMode()) {
+              checkLeases();
+            }
+          } finally {
+            fsnamesystem.writeUnlock();
           }
-        } finally {
-          fsnamesystem.writeUnlock();
-        }
   
   
-        try {
           Thread.sleep(HdfsServerConstants.NAMENODE_LEASE_RECHECK_INTERVAL);
         } catch(InterruptedException ie) {
           if (LOG.isDebugEnabled()) {
@@ -437,4 +444,36 @@ public class LeaseManager {
         + "\n sortedLeasesByPath=" + sortedLeasesByPath
         + "\n}";
   }
+
+  void startMonitor() {
+    Preconditions.checkState(lmthread == null,
+        "Lease Monitor already running");
+    shouldRunMonitor = true;
+    lmthread = new Daemon(new Monitor());
+    lmthread.start();
+  }
+  
+  void stopMonitor() {
+    if (lmthread != null) {
+      shouldRunMonitor = false;
+      try {
+        lmthread.interrupt();
+        lmthread.join(3000);
+      } catch (InterruptedException ie) {
+        LOG.warn("Encountered exception ", ie);
+      }
+      lmthread = null;
+    }
+  }
+
+  /**
+   * Trigger the currently-running Lease monitor to re-check
+   * its leases immediately. This is for use by unit tests.
+   */
+  @VisibleForTesting
+  void triggerMonitorCheckNow() {
+    Preconditions.checkState(lmthread != null,
+        "Lease monitor is not running");
+    lmthread.interrupt();
+  }
 }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/util/RwLock.java

@@ -31,6 +31,9 @@ public interface RwLock {
   /** Acquire write lock. */
   public void writeLock();
   
+  /** Acquire write lock, unless interrupted while waiting  */
+  void writeLockInterruptibly() throws InterruptedException;
+
   /** Release write lock. */
   public void writeUnlock();
 

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java

@@ -78,7 +78,7 @@ public class NameNodeAdapter {
   /** Set the softLimit and hardLimit of client lease periods. */
   public static void setLeasePeriod(final FSNamesystem namesystem, long soft, long hard) {
     getLeaseManager(namesystem).setLeasePeriod(soft, hard);
-    namesystem.lmthread.interrupt();
+    namesystem.leaseManager.triggerMonitorCheckNow();
   }
 
   public static String getLeaseHolderForPath(NameNode namenode, String path) {