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

D M Murali Krishna Reddy commented on YARN-10745:
-------------------------------------------------

Thanks [~ebadger]  for the review comments.

Regarding the wrong indentation, In the initial patches I have followed the 
correct level of indentation but the hadoopQA checkstyle was showing error, So 
I have changed the indentation to fix the checkstyle warnings. I will change 
the indentation level as per your review.

 

Is {{clusterNodeReports}} guaranteeed to be non-null here?

Yes, as per my understanding the clusterNodeReports will never be null. If it 
was null we would be getting a NPE in the below for loop anyway. Also, I think 
findbugs would catch this type of potential NPE, So I don't think it is a 
problem.

 
{code:java}
-    // NodeManager is the last service to start, so NodeId is available.
+    // NodeStatusUpdater is the last service to start, so NodeId is available.

{code}
Regarding the above change, I have misunderstood the old comment and changed 
it. Will be reverting it.

 
{code:java}
+      LOG.info("Callback succeeded for initializing request processing " +
+          "pipeline for an AM ");
{code}
I haven't debugged AMRMProxy a lot, but going through the code found it might 
be useful to have this log. If you feel it is not required and doesn't add any 
value, I can remove it.

 
{code:java}
-    LOG.info("hostsReader include:{" +
-        StringUtils.join(",", hostsReader.getHosts()) +
-        "} exclude:{" +
-        StringUtils.join(",", hostsReader.getExcludedHosts()) + "}");
-
+    if (!hostsReader.getHosts().isEmpty() ||
+        !hostsReader.getExcludedHosts().isEmpty()) {
+      LOG.info("hostsReader include:{" +
+          StringUtils.join(",", hostsReader.getHosts()) +
+          "} exclude:{" +
+          StringUtils.join(",", hostsReader.getExcludedHosts()) + "}");
+    }
{code}
I have added this change as per the suggestion of [~BilwaST], I will remove 
this change in the 005 patch.

 

> Change Log level from info to debug for few logs and remove unnecessary 
> debuglog checks
> ---------------------------------------------------------------------------------------
>
>                 Key: YARN-10745
>                 URL: https://issues.apache.org/jira/browse/YARN-10745
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: D M Murali Krishna Reddy
>            Assignee: D M Murali Krishna Reddy
>            Priority: Minor
>         Attachments: YARN-10745.001.patch, YARN-10745.002.patch, 
> YARN-10745.003.patch, YARN-10745.004.patch
>
>
> Change the info log level to debug for few logs so that the load on the 
> logger decreases in large cluster and improves the performance.
> Remove the unnecessary isDebugEnabled() checks for printing strings without 
> any string concatenation



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to