HDFS-14256. Review Logging of NameNode Class. Contributed by David Mollitor.

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

@@ -111,6 +111,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.TreeSet;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
@@ -1034,7 +1035,7 @@ public class NameNode extends ReconfigurableBase implements
     try {
       rpcServer.join();
     } catch (InterruptedException ie) {
-      LOG.info("Caught interrupted exception ", ie);
+      LOG.info("Caught interrupted exception", ie);
     }
   }
 
@@ -1052,7 +1053,7 @@ public class NameNode extends ReconfigurableBase implements
         state.exitState(haContext);
       }
     } catch (ServiceFailedException e) {
-      LOG.warn("Encountered exception while exiting state ", e);
+      LOG.warn("Encountered exception while exiting state", e);
     } finally {
       stopMetricsLogger();
       stopCommonServices();
@@ -1211,7 +1212,8 @@ public class NameNode extends ReconfigurableBase implements
       //Generate a new cluster id
       clusterId = NNStorage.newClusterID();
     }
-    System.out.println("Formatting using clusterid: " + clusterId);
+
+    LOG.info("Formatting using clusterid: {}", clusterId);
     
     FSImage fsImage = new FSImage(conf, nameDirsToFormat, editDirsToFormat);
     try {
@@ -1239,7 +1241,7 @@ public class NameNode extends ReconfigurableBase implements
 
       fsImage.format(fsn, clusterId, force);
     } catch (IOException ioe) {
-      LOG.warn("Encountered exception during format: ", ioe);
+      LOG.warn("Encountered exception during format", ioe);
       fsImage.close();
       throw ioe;
     }
@@ -1400,13 +1402,11 @@ public class NameNode extends ReconfigurableBase implements
 
       // Copy all edits after last CheckpointTxId to shared edits dir
       for (EditLogInputStream stream : streams) {
-        LOG.debug("Beginning to copy stream " + stream + " to shared edits");
+        LOG.debug("Beginning to copy stream {} to shared edits", stream);
         FSEditLogOp op;
         boolean segmentOpen = false;
         while ((op = stream.readOp()) != null) {
-          if (LOG.isTraceEnabled()) {
-            LOG.trace("copying op: " + op);
-          }
+          LOG.trace("copying op: {}", op);
           if (!segmentOpen) {
             newSharedEditLog.startLogSegment(op.txid, false,
                 fsns.getEffectiveLayoutVersion());
@@ -1417,14 +1417,15 @@ public class NameNode extends ReconfigurableBase implements
 
           if (op.opCode == FSEditLogOpCodes.OP_END_LOG_SEGMENT) {
             newSharedEditLog.endCurrentLogSegment(false);
-            LOG.debug("ending log segment because of END_LOG_SEGMENT op in "
-                + stream);
+            LOG.debug("ending log segment because of END_LOG_SEGMENT op in {}",
+                stream);
             segmentOpen = false;
           }
         }
 
         if (segmentOpen) {
-          LOG.debug("ending log segment because of end of stream in " + stream);
+          LOG.debug("ending log segment because of end of stream in {}",
+              stream);
           newSharedEditLog.logSync();
           newSharedEditLog.endCurrentLogSegment(false);
           segmentOpen = false;
@@ -1541,7 +1542,7 @@ public class NameNode extends ReconfigurableBase implements
               i += 1;
             }
           } else {
-            LOG.error("Unknown upgrade flag " + flag);
+            LOG.error("Unknown upgrade flag: {}", flag);
             return null;
           }
         }
@@ -1685,8 +1686,8 @@ public class NameNode extends ReconfigurableBase implements
       terminate(aborted ? 1 : 0);
       return null; // avoid javac warning
     case GENCLUSTERID:
-      System.err.println("Generating new cluster id:");
-      System.out.println(NNStorage.newClusterID());
+      String clusterID = NNStorage.newClusterID();
+      LOG.info("Generated new cluster id: {}", clusterID);
       terminate(0);
       return null;
     case ROLLBACK:
@@ -1767,9 +1768,7 @@ public class NameNode extends ReconfigurableBase implements
       URI defaultUri = URI.create(HdfsConstants.HDFS_URI_SCHEME + "://"
           + conf.get(DFS_NAMENODE_RPC_ADDRESS_KEY));
       conf.set(FS_DEFAULT_NAME_KEY, defaultUri.toString());
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Setting " + FS_DEFAULT_NAME_KEY + " to " + defaultUri.toString());
-      }
+      LOG.debug("Setting {} to {}", FS_DEFAULT_NAME_KEY, defaultUri);
     }
   }
     
@@ -1906,22 +1905,14 @@ public class NameNode extends ReconfigurableBase implements
 
   @Override // NameNodeStatusMXBean
   public String getNNRole() {
-    String roleStr = "";
     NamenodeRole role = getRole();
-    if (null != role) {
-      roleStr = role.toString();
-    }
-    return roleStr;
+    return Objects.toString(role, "");
   }
 
   @Override // NameNodeStatusMXBean
   public String getState() {
-    String servStateStr = "";
     HAServiceState servState = getServiceState();
-    if (null != servState) {
-      servStateStr = servState.toString();
-    }
-    return servStateStr;
+    return Objects.toString(servState, "");
   }
 
   @Override // NameNodeStatusMXBean
@@ -1967,10 +1958,9 @@ public class NameNode extends ReconfigurableBase implements
    */
   protected synchronized void doImmediateShutdown(Throwable t)
       throws ExitException {
-    String message = "Error encountered requiring NN shutdown. " +
-        "Shutting down immediately.";
     try {
-      LOG.error(message, t);
+      LOG.error("Error encountered requiring NN shutdown. " +
+          "Shutting down immediately.", t);
     } catch (Throwable ignored) {
       // This is unlikely to happen, but there's nothing we can do if it does.
     }