[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14376493#comment-14376493 ] Bikas Saha commented on TEZ-2176: - There should probably be a follow up jira to remove instances of LOG.isDebugEnabled() from the code based on http://www.slf4j.org/faq.html#logging_performance [~vasanthkumar] Do you think you can take a crack at it? > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Fix For: 0.7.0 > > Attachments: TEZ-2176.1.patch, TEZ-2176.2.1.txt, TEZ-2176.2.patch, > TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14376404#comment-14376404 ] Siddharth Seth commented on TEZ-2176: - +1. Looks good. Attaching a rebased patch after the last few commits and committing, before this goes stale. Thanks [~vasanthkumar]. > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.1.patch, TEZ-2176.2.1.txt, TEZ-2176.2.patch, > TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[
https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14375150#comment-14375150
]
Hadoop QA commented on TEZ-2176:
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12706408/TEZ-2176.2.patch
against master revision a44f5c5.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 44 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/325//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/325//console
This message is automatically generated.
> Move all logging to slf4j
> -
>
> Key: TEZ-2176
> URL: https://issues.apache.org/jira/browse/TEZ-2176
> Project: Apache Tez
> Issue Type: Improvement
>Reporter: Siddharth Seth
>Assignee: Vasanth kumar RJ
> Attachments: TEZ-2176.1.patch, TEZ-2176.2.patch, TEZ-2176.patch
>
>
> SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings.
> Also drop commons-logging from the dependency set.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14366413#comment-14366413 ] Siddharth Seth commented on TEZ-2176: - [~vasanthkumar] - thanks for pointing out the limitation on markers. Missed that bit. I think we're fine moving these messages to ERROR in that case, instead of depending on log4j directly. Regarding the version change, lets not do that in this jira. There may be additional bits to look at where we actually change log files using log4j APIs directly. [~hitesh] - the 2.x change will be a separate jira. Will need some testing to see what happens if log4j2 and log4j1.x are on the classpath. Also the slf4j-lo4j12 / slf4j-log4j2 bindings. We can control the version shipped by Tez - but these may come in externally, in which case some testing will be required. > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.1.patch, TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14366322#comment-14366322 ] Hitesh Shah commented on TEZ-2176: -- [~sseth] Can you confirm/clarify that these changes will not affect any user code that continues to use log4j 1.x? > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.1.patch, TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14365868#comment-14365868 ] Vasanth kumar RJ commented on TEZ-2176: --- [~sseth], Above comment link says fatal can be implemented using Marker interface only for logback and not for log4j/java logging. In your previous comment, you suggested to use hadoop's log4j version. Combination of projects using log4j and log4j2 may not be compatible. > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.1.patch, TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14365630#comment-14365630 ] Siddharth Seth commented on TEZ-2176: - [~vasanthkumar] - thanks for making the change. Instead of using log4j directrly - can we use slf4j markers for fatal. http://slf4j.org/faq.html#fatal . The rest looks good. We may as well move to log4j2 as well - which I believe offers better performance. I'll create a follow up for this. Should be a straighforward patch. > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.1.patch, TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[
https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14359410#comment-14359410
]
Hadoop QA commented on TEZ-2176:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12704242/TEZ-2176.1.patch
against master revision 63e985d.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 43 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in :
org.apache.tez.test.TestAMRecovery
Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/287//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/287//console
This message is automatically generated.
> Move all logging to slf4j
> -
>
> Key: TEZ-2176
> URL: https://issues.apache.org/jira/browse/TEZ-2176
> Project: Apache Tez
> Issue Type: Improvement
>Reporter: Siddharth Seth
>Assignee: Vasanth kumar RJ
> Attachments: TEZ-2176.1.patch, TEZ-2176.patch
>
>
> SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings.
> Also drop commons-logging from the dependency set.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14357708#comment-14357708 ] Siddharth Seth commented on TEZ-2176: - Thanks [~vasanthkumar]. I think we should stick to the version hadoop exports - to avoid the warning. Checked hadoop trunk, and that uses 1.7.5 as well. Do you see any problems with using this version ? Otherwise, could you please revise the patch. I'll create a follow up to change all the files and remove the jcl-over-slf4j dependency. Hoping it's a simple set of sed commands to change all the files (except for any LOG.fatal messages that we have). > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14357411#comment-14357411 ] Vasanth kumar RJ commented on TEZ-2176: --- [~sseth], below given my comments Couple of questions: is the log4j12 binding dependency required along with the slf4j-log4j12 dependency, or can that be dropped ? - slf4j-log4j12 has log4j dependency. So Log4j will be underlying logging framework. If I understand this correctly, jcl-over-slf4j provides the same interface as commons-logging - which is why the code doesn't need to change to explicitly use the slf4j interfaces ? - Yes. Reference: http://www.slf4j.org/legacy.html The classpath for the AM / container will often container additional jars defined by users - in which case commons-logging.jar may show up. In this case, we shouldn't have any trouble right ? Either jcl-over-slf4j will be used or commons-logging - but this shouldn't cause failures ? - I tried with both the jars and any one of the jar in classpath. I didn't get any errors/failures. Hadoop exports slf4j-1.7.5. If jars from the hadoop classpath are included for Tez containers, will this cause problems ? - When multiple version of slf4j-log4j are found then prints warning message and user has to remove. Eventually, I think we should move the Loggers in each file to use slf4j directly. - We have to change ~190 files for avoiding jcl-over-slf4j > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[ https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14355623#comment-14355623 ] Siddharth Seth commented on TEZ-2176: - Thanks for posting the patch [~vasanthkumar]. Mostly looks good to me. Couple of questions: is the log4j12 binding dependency required along with the slf4j-log4j12 dependency, or can that be dropped ? If I understand this correctly, jcl-over-slf4j provides the same interface as commons-logging - which is why the code doesn't need to change to explicitly use the slf4j interfaces ? The classpath for the AM / container will often container additional jars defined by users - in which case commons-logging.jar may show up. In this case, we shouldn't have any trouble right ? Either jcl-over-slf4j will be used or commons-logging - but this shouldn't cause failures ? Hadoop exports slf4j-1.7.5. If jars from the hadoop classpath are included for Tez containers, will this cause problems ? Eventually, I think we should move the Loggers in each file to use slf4j directly. [~hitesh] - do you see any other issues with this change. I think we'll have to mark it as incompatible since commons-logging is no longer included. > Move all logging to slf4j > - > > Key: TEZ-2176 > URL: https://issues.apache.org/jira/browse/TEZ-2176 > Project: Apache Tez > Issue Type: Improvement >Reporter: Siddharth Seth >Assignee: Vasanth kumar RJ > Attachments: TEZ-2176.patch > > > SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings. > Also drop commons-logging from the dependency set. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2176) Move all logging to slf4j
[
https://issues.apache.org/jira/browse/TEZ-2176?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14352792#comment-14352792
]
Hadoop QA commented on TEZ-2176:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12703397/TEZ-2176.patch
against master revision 4a19a43.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/266//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/266//console
This message is automatically generated.
> Move all logging to slf4j
> -
>
> Key: TEZ-2176
> URL: https://issues.apache.org/jira/browse/TEZ-2176
> Project: Apache Tez
> Issue Type: Improvement
>Reporter: Siddharth Seth
>Assignee: Vasanth kumar RJ
> Attachments: TEZ-2176.patch
>
>
> SLF4J supports a more comprehensive set of APIs - MDC, Formatted strings.
> Also drop commons-logging from the dependency set.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
