[
https://issues.apache.org/jira/browse/YARN-9343?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16783952#comment-16783952
]
Wilfred Spiegelenburg commented on YARN-9343:
---------------------------------------------
Hi [~Prabhu Joseph], thank you for this update. I have looked at it a couple of
times and just updated the parts that I touched. It is good to have this done
globally.
I do have some remarks:
* I saw an inconsistency in how we log exception. In some places we use
{{debug(ex.getMessage());}} while in other we just use {{debug({}, ex);}} would
be good to come to a standard way of logging them.
* Again for consistency sake: in the case that we just log the exception it
would be nice to add that to the message text itself so we know that it is
ignored, we do it in a number of places but not everywhere.
* In {{CombinedResourceCalculator}} we have two consecutive LOG.debug
statements in the diff, only one is replaced.
* Do we need to use {{String.valueOf(pullImageTimeMs)}} in
{{DockerLinuxContainerRuntime}} can we not just pass the object?
* In {{ResourceLocalizationService}} you have missed a object reference in the
text:
{code:java}
LOG.debug("Skip downloading resource: {} since it's in"
+ " state: ", key, rsrc.getState());
{code}
* In {{AmIpFilter}} you have removed the guard but not changed the format
string etc.
{code}
LOG.debug("Could not find " + WebAppProxyServlet.PROXY_USER_COOKIE_NAME
+ " cookie, so user will not be set");
{code}
I saw a couple of cases in which we are doing expensive operations in preparing
the objects just for logging. Should we not keep the guard around them to
prevent the overhead:
* TimelineUtils.dumpTimelineRecordtoJSON(entity)
* Arrays.toString(fullCommandArray)
* StringUtils.join(",", assignedResources)
Can you also check the checkstyle issues and clean up the line breaks string
concats you are using?
> Replace isDebugEnabled with SLF4J parameterized log messages
> ------------------------------------------------------------
>
> Key: YARN-9343
> URL: https://issues.apache.org/jira/browse/YARN-9343
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Prabhu Joseph
> Assignee: Prabhu Joseph
> Priority: Major
> Attachments: YARN-9343-001.patch
>
>
> Replace isDebugEnabled with SLF4J parameterized log messages.
> https://www.slf4j.org/faq.html
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
