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/branches/branch-0.23@1490490 13f79535-47bb-0310-9956-ffa450edef68

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

@@ -12,6 +12,8 @@ Release 0.23.9 - UNRELEASED
 
   BUG FIXES
 
+    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

@@ -2954,7 +2954,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);
@@ -3150,11 +3149,10 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     }
 
     /**
-     * Check if safe mode was entered manually or automatically (at startup, or
-     * when disk space is low).
+     * Check if safe mode was entered manually
      */
     private boolean isManual() {
-      return extension == Integer.MAX_VALUE && !resourcesLow;
+      return extension == Integer.MAX_VALUE;
     }
 
     /**
@@ -3190,7 +3188,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       } else {
         leaveMsg = "Safe mode will be turned off automatically";
       }
-      if(isManual()) {
+      if(isManual() && !areResourcesLow()) {
         if(upgradeManager.getUpgradeState())
           return leaveMsg + " upon completion of " + 
             "the distributed upgrade: upgrade progress = " + 
@@ -3232,7 +3230,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
@@ -3349,7 +3348,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
@@ -3437,7 +3441,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 {
@@ -3453,8 +3457,9 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
       }
       if (resourcesLow) {
         safeMode.setResourcesLow();
+      } else {
+        safeMode.setManual();
       }
-      safeMode.setManual();
       getEditLog().logSyncAll();
       NameNode.stateChangeLog.info("STATE* Safe mode is ON. " 
                                   + safeMode.getTurnOffTip());

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

@@ -0,0 +1,54 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.apache.hadoop.conf.Configuration;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class TestFSNamesystem {
+  @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(fsImage, conf);
+
+    fsn.leaveSafeMode(false);
+    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());
+  }
+}