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