jojochuang commented on code in PR #5315:
URL: https://github.com/apache/hadoop/pull/5315#discussion_r1087030863


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java:
##########
@@ -360,7 +358,8 @@ public class DataNode extends ReconfigurableBase
               FS_GETSPACEUSED_JITTER_KEY,
               FS_GETSPACEUSED_CLASSNAME));
 
-  public static final Log METRICS_LOG = 
LogFactory.getLog("DataNodeMetricsLog");
+  public static final org.apache.log4j.Logger METRICS_LOG =
+      org.apache.log4j.Logger.getLogger("DataNodeMetricsLog");

Review Comment:
   We should avoid using log4j directly. The proper usage is to use slf4j 
Logger, and it will then indirectly use log4j. In fact ideally we don't even 
need to declare direct dependency on log4.  Certain logger usage in Hadoop 
forces us to use log4j APIs but in generally that's discouraged.
   I don't mind making the follow up change later. This PR is already huge and 
take too much time to iterate.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/LogAdapter.java:
##########
@@ -17,62 +17,34 @@
  */
 package org.apache.hadoop.util;
 
-import org.apache.commons.logging.Log;
 import org.slf4j.Logger;
 
-class LogAdapter {
-  private Log LOG;
-  private Logger LOGGER;
+@Deprecated
+final class LogAdapter {

Review Comment:
   I wonder what the purpose of this class. Looks like it can be simply 
replaced with using Logger directly, I think.
   maybe it can removed later.



##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java:
##########
@@ -491,8 +488,7 @@ private boolean isClientPortInfoAbsent(CallerContext ctx){
    * perm=<permissions (optional)>
    * </code>
    */
-  public static final Log auditLog = LogFactory.getLog(
-      FSNamesystem.class.getName() + ".audit");
+  public static final Logger AUDIT_LOG = 
Logger.getLogger(FSNamesystem.class.getName() + ".audit");

Review Comment:
   As long as the NameNode audit logger does not break (there are tests that 
covers it) I am ok.



##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/IOUtils.java:
##########
@@ -256,7 +255,7 @@ public static void skipFully(InputStream in, long len) 
throws IOException {
    * instead
    */
   @Deprecated
-  public static void cleanup(Log log, java.io.Closeable... closeables) {
+  public static void cleanup(Logger log, java.io.Closeable... closeables) {

Review Comment:
   Agreed we should not remove it now. Likely to break applications.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to