hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Varun Saxena (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-5450) Enhance logging for Cluster.java around InetSocketAddress
Date Sat, 06 Aug 2016 15:55:20 GMT

    [ https://issues.apache.org/jira/browse/YARN-5450?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15410641#comment-15410641
] 

Varun Saxena edited comment on YARN-5450 at 8/6/16 3:55 PM:
------------------------------------------------------------

[~vrushalic], thanks for the patch.
Will move this JIRA to Mapreduce project as the change is strictly in MapReduce project.

Few comments:
# In my opinion, there is no need to check LOG#isInfoEnabled because logs are typically at
INFO level anyway. Our codebase if full of logging at INFO level without the check.
# I am not 100% sure of the use case but I think we should move the log statement above the
check for clientprotocolprovider being null because exception is thrown there. And this log
will be more useful if a  clientprotocolprovider implementation cannot be picked. Maybe move
this log above the for loop ?
{code}
138	    if (null == clientProtocolProvider || null == client) {
139	      throw initEx;
140	    }
141	    if (LOG.isInfoEnabled() && jobTrackAddr != null) {
142	      LOG.info("Initialized Cluster for source=" + jobTrackAddr.toString());
143	    }
{code}

  3. Change {{LOG.info("Initializing Cluster for source=}} to {{LOG.info("Initializing Cluster
for job tracker }} ?


was (Author: varun_saxena):
[~vrushalic], thanks for the patch.
Will move this JIRA to Mapreduce project as the change is strictly in MapReduce project.

A couple of comments:
# In my opinion, there is no need to check LOG#isInfoEnabled because logs are typically at
INFO level anyway. Our codebase if full of logging at INFO level without the check.
# I am not 100% sure of the use case but I think we should move the log statement above the
check for clientprotocolprovider being null because exception is thrown there. And this log
will be more useful if a  clientprotocolprovider implementation cannot be picked. Maybe move
this log above the for loop ?
{code}
138	    if (null == clientProtocolProvider || null == client) {
139	      throw initEx;
140	    }
141	    if (LOG.isInfoEnabled() && jobTrackAddr != null) {
142	      LOG.info("Initialized Cluster for source=" + jobTrackAddr.toString());
143	    }
{code}

3. Change {{LOG.info("Initializing Cluster for source=}} to {{LOG.info("Initializing Cluster
for job tracker }} ?

> Enhance logging for Cluster.java around InetSocketAddress
> ---------------------------------------------------------
>
>                 Key: YARN-5450
>                 URL: https://issues.apache.org/jira/browse/YARN-5450
>             Project: Hadoop YARN
>          Issue Type: New Feature
>          Components: yarn
>            Reporter: sarun singla
>            Assignee: Vrushali C
>            Priority: Minor
>              Labels: YARN
>         Attachments: YARN-5450.01.patch
>
>
> We need to add more logging for cluster.java class around " initialize(InetSocketAddress
jobTrackAddr, Configuration conf) " method to give better logging like about the source of
the property.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message