HDFS-14859. Prevent unnecessary evaluation of costly operation getNumLiveDataNodes when dfs.namenode.safemode.min.datanodes is not zero. Contributed by Srinivasu Majeti.

Author: ayushtkn

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java

@@ -572,12 +572,18 @@ private boolean areThresholdsMet() {
     assert namesystem.hasWriteLock();
     // Calculating the number of live datanodes is time-consuming
     // in large clusters. Skip it when datanodeThreshold is zero.
-    int datanodeNum = 0;
-    if (datanodeThreshold > 0) {
-      datanodeNum = blockManager.getDatanodeManager().getNumLiveDataNodes();
-    }
+    // We need to evaluate getNumLiveDataNodes only when
+    // (blockSafe >= blockThreshold) is true and hence moving evaluation
+    // of datanodeNum conditional to isBlockThresholdMet as well
     synchronized (this) {
-      return blockSafe >= blockThreshold && datanodeNum >= datanodeThreshold;
+      boolean isBlockThresholdMet = (blockSafe >= blockThreshold);
+      boolean isDatanodeThresholdMet = true;
+      if (isBlockThresholdMet && datanodeThreshold > 0) {
+        int datanodeNum = blockManager.getDatanodeManager().
+                getNumLiveDataNodes();
+        isDatanodeThresholdMet = (datanodeNum >= datanodeThreshold);
+      }
+      return isBlockThresholdMet && isDatanodeThresholdMet;
     }
   }
 

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockManagerSafeMode.java

@@ -377,6 +377,87 @@ public void testDatanodeThreshodShouldBeMet() throws Exception {
     assertFalse(bmSafeMode.isInSafeMode());
   }
 
+  /**
+   * Test block manager won't leave safe mode if datanode threshold is not met
+   * only if datanodeThreshold is configured > 0.
+   */
+  @Test(timeout = 30000)
+  public void testDatanodeThreshodShouldBeMetOnlyIfConfigured()
+          throws Exception {
+    bmSafeMode.activate(BLOCK_TOTAL);
+
+    //Blocks received is set to threshold
+    setBlockSafe(BLOCK_THRESHOLD);
+
+    //datanodeThreshold is configured to 1 but not all DNs registered .
+    // Expecting safe mode .
+    when(dn.getNumLiveDataNodes()).thenReturn(1);
+    setDatanodeThreshold(1);
+    bmSafeMode.checkSafeMode();
+    assertTrue(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 1 and all DNs registered .
+    // Not expecting safe mode .
+    when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM);
+    setDatanodeThreshold(1);
+    bmSafeMode.checkSafeMode();
+    waitForExtensionPeriod();
+    assertFalse(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 0 but not all DNs registered .
+    // Not Expecting safe mode .
+    bmSafeMode.activate(BLOCK_TOTAL);
+    setBlockSafe(BLOCK_THRESHOLD);
+    when(dn.getNumLiveDataNodes()).thenReturn(1);
+    setDatanodeThreshold(0);
+    bmSafeMode.checkSafeMode();
+    assertFalse(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 0 and all DNs registered .
+    // Not Expecting safe mode .
+    bmSafeMode.activate(BLOCK_TOTAL);
+    setBlockSafe(BLOCK_THRESHOLD);
+    when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM);
+    setDatanodeThreshold(0);
+    bmSafeMode.checkSafeMode();
+    assertFalse(bmSafeMode.isInSafeMode());
+
+    //Blocks received set to below threshold and all combinations
+    //of datanodeThreshold should result in safe mode.
+
+
+    //datanodeThreshold is configured to 1 but not all DNs registered .
+    // Expecting safe mode .
+    bmSafeMode.activate(BLOCK_TOTAL);
+    setBlockSafe(BLOCK_THRESHOLD-1);
+    setSafeModeStatus(BMSafeModeStatus.PENDING_THRESHOLD);
+    when(dn.getNumLiveDataNodes()).thenReturn(1);
+    setDatanodeThreshold(1);
+    bmSafeMode.checkSafeMode();
+    assertTrue(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 1 and all DNs registered .
+    // Expecting safe mode .
+    when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM);
+    setDatanodeThreshold(1);
+    bmSafeMode.checkSafeMode();
+    assertTrue(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 0 but not all DNs registered .
+    // Expecting safe mode .
+    when(dn.getNumLiveDataNodes()).thenReturn(1);
+    setDatanodeThreshold(0);
+    bmSafeMode.checkSafeMode();
+    assertTrue(bmSafeMode.isInSafeMode());
+
+    //datanodeThreshold is configured to 0 and all DNs registered .
+    // Expecting safe mode .
+    when(dn.getNumLiveDataNodes()).thenReturn(DATANODE_NUM);
+    setDatanodeThreshold(0);
+    bmSafeMode.checkSafeMode();
+    assertTrue(bmSafeMode.isInSafeMode());
+  }
+
   /**
    * Test block manager won't leave safe mode if there are blocks with
    * generation stamp (GS) in future.
@@ -604,6 +685,11 @@ private void setBlockSafe(long blockSafe) {
     Whitebox.setInternalState(bmSafeMode, "blockSafe", blockSafe);
   }
 
+  private void setDatanodeThreshold(int dnSafeModeThreshold) {
+    Whitebox.setInternalState(bmSafeMode, "datanodeThreshold",
+            dnSafeModeThreshold);
+  }
+
   private long getblockSafe() {
     return (long)Whitebox.getInternalState(bmSafeMode, "blockSafe");
   }