HDFS-4862. SafeModeInfo.isManual() returns true when resources are low even if it wasn't entered into manually. Contributed by Ravi Prakash.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1490486 13f79535-47bb-0310-9956-ffa450edef68

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

@@ -3117,6 +3117,9 @@ Release 0.23.9 - UNRELEASED
     HDFS-4867. metaSave NPEs when there are invalid blocks in repl queue.
     (Plamen Jeliazkov and Ravi Prakash via shv)
 
+    HDFS-4862. SafeModeInfo.isManual() returns true when resources are low even
+    if it wasn't entered into manually (Ravi Prakash via kihwal)
+
 Release 0.23.8 - 2013-06-05
 
   INCOMPATIBLE CHANGES

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

@@ -4101,7 +4101,6 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       this.replQueueThreshold = 1.5f; // can never be reached
       this.blockTotal = -1;
       this.blockSafe = -1;
-      this.reached = -1;
       this.resourcesLow = resourcesLow;
       enter();
       reportStatus("STATE* Safe mode is ON.", true);
@@ -4288,17 +4287,17 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     private synchronized void decrementSafeBlockCount(short replication) {
       if (replication == safeReplication-1) {
         this.blockSafe--;
-        assert blockSafe >= 0 || isManual();
+        //blockSafe is set to -1 in manual / low resources safemode
+        assert blockSafe >= 0 || isManual() || areResourcesLow();
         checkMode();
       }
     }
 
     /**
-     * Check if safe mode was entered manually or automatically (at startup, or
+     * Check if safe mode was entered manually
-     * when disk space is low).
      */
     private boolean isManual() {
-      return extension == Integer.MAX_VALUE && !resourcesLow;
+      return extension == Integer.MAX_VALUE;
     }
 
     /**
@@ -4337,7 +4336,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       } else {
         leaveMsg = "Safe mode will be turned off automatically";
       }
-      if(isManual()) {
+      if(isManual() && !areResourcesLow()) {
         leaveMsg = "Use \"hdfs dfsadmin -safemode leave\" to turn safe mode off";
       }
 
@@ -4375,7 +4374,8 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
         }
         msg += " " + leaveMsg;
       }
-      if(reached == 0 || isManual()) {  // threshold is not reached or manual       
+      // threshold is not reached or manual or resources low
+      if(reached == 0 || (isManual() && !areResourcesLow())) {
         return msg + ".";
       }
       // extension period is in progress
@@ -4519,7 +4519,12 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     SafeModeInfo safeMode = this.safeMode;
     if (safeMode == null)
       return false;
-    return !safeMode.isManual() && safeMode.isOn();
+    // If the NN is in safemode, and not due to manual / low resources, we
+    // assume it must be because of startup. If the NN had low resources during
+    // startup, we assume it came out of startup safemode and it is now in low
+    // resources safemode
+    return !safeMode.isManual() && !safeMode.areResourcesLow()
+      && safeMode.isOn();
   }
 
   @Override
@@ -4632,7 +4637,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
   }
 
   /**
-   * Enter safe mode manually.
+   * Enter safe mode. If resourcesLow is false, then we assume it is manual
    * @throws IOException
    */
   void enterSafeMode(boolean resourcesLow) throws IOException {
@@ -4657,8 +4662,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       }
       if (resourcesLow) {
         safeMode.setResourcesLow();
-      }
+      } else {
         safeMode.setManual();
+      }
       if (isEditlogOpenForWrite) {
         getEditLog().logSyncAll();
       }

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

@@ -21,6 +21,7 @@ package org.apache.hadoop.hdfs.server.namenode;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_EDITS_DIR_KEY;
 import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_NAMENODE_NAME_DIR_KEY;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.IOException;
@@ -35,6 +36,7 @@ import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.server.common.HdfsServerConstants.NamenodeRole;
 import org.junit.After;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 public class TestFSNamesystem {
 
@@ -77,4 +79,29 @@ public class TestFSNamesystem {
     leaseMan = fsn.getLeaseManager();
     assertEquals(0, leaseMan.countLease());
   }
+
+  @Test
+  /**
+   * Test that isInStartupSafemode returns true only during startup safemode
+   * and not also during low-resource safemode
+   */
+  public void testStartupSafemode() throws IOException {
+    Configuration conf = new Configuration();
+    FSImage fsImage = Mockito.mock(FSImage.class);
+    FSEditLog fsEditLog = Mockito.mock(FSEditLog.class);
+    Mockito.when(fsImage.getEditLog()).thenReturn(fsEditLog);
+    FSNamesystem fsn = new FSNamesystem(conf, fsImage);
+
+    fsn.leaveSafeMode();
+    assertTrue("After leaving safemode FSNamesystem.isInStartupSafeMode still "
+      + "returned true", !fsn.isInStartupSafeMode());
+    assertTrue("After leaving safemode FSNamesystem.isInSafeMode still returned"
+      + " true", !fsn.isInSafeMode());
+
+    fsn.enterSafeMode(true);
+    assertTrue("After entering safemode due to low resources FSNamesystem."
+      + "isInStartupSafeMode still returned true", !fsn.isInStartupSafeMode());
+    assertTrue("After entering safemode due to low resources FSNamesystem."
+      + "isInSafeMode still returned false",  fsn.isInSafeMode());
+  }
 }