[
https://issues.apache.org/jira/browse/TEZ-4095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17162970#comment-17162970
]
Jonathan Turner Eagles commented on TEZ-4095:
---------------------------------------------
Another reason to use isDebugEnabled guards is for performance reason. In this
way we can avoid creation of expensive toString calculation and creation of
temporary strings. The following changes fall under this category and need
another change.
dagId.toString is expensive and should not be computed unless debug enabled
{code:title=DAGAppMaster}
@@ -1030,9 +1028,8 @@ public class DAGAppMaster extends AbstractService {
try {
if (LOG.isDebugEnabled()) {
- LOG.debug("JSON dump for submitted DAG, dagId=" + dagId.toString()
- + ", json="
- + DAGUtils.generateSimpleJSONPlan(dagPB).toString());
+ LOG.debug("JSON dump for submitted DAG, dagId=" + dagId + ", json="
+ + DAGUtils.generateSimpleJSONPlan(dagPB));
}
{code}
ServiceThread.getName != ServiceThread.toString so what is logged is not the
same.
{code}
- if(LOG.isDebugEnabled()) {
- LOG.debug("Waiting for service thread to join for " + st.getName());
- }
+ LOG.debug("Waiting for service thread to join for {}", st);
{code}
Should be a space between outputName, and logIdentifier
{code:title=VertexImpl}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Ignoring committer as none specified for output="
- + outputName
- + ", vertexId=" + logIdentifier);
- }
+ LOG.debug("Ignoring committer as none specified for output={},
vertexId={}",
+ outputName,logIdentifier);
{code}
task != task.getTaskId (logic change here and other places) same for attempt !=
attempt.getID in TaskImpl
{code}
- if(LOG.isDebugEnabled()) {
- LOG.debug("Created task for vertex " + logIdentifier + ": " +
- task.getTaskId());
- }
+ LOG.debug("Created task for vertex {}: {}", logIdentifier, task);
{code}
Again this is a logic change. Best to keep all logging the same and avoid all
logic changes.
{code}
+ LOG.debug("Removing task: {}", entry);
{code}
Same statements are not based on the same condition as before.
{code:title=YarnTaskSchedulerService}
- if (LOG.isDebugEnabled()) {
- LOG.debug(highestPriRequest + " fits in free resources");
- } else {
- if (numHeartbeats % 50 == 1) {
- LOG.info(highestPriRequest + " fits in free resources");
- }
+
+ LOG.debug("{} fits in free resources", highestPriRequest);
+ if (numHeartbeats % 50 == 1) {
+ LOG.info(highestPriRequest + " fits in free resources");
}
{code}
Please change back to container.getId
{code}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Releasing unused container: " + container.getId());
- }
+ LOG.debug("Releasing unused container: {}", container);
{code}
eventType.name not necessarily the same as eventType.toString (in this case it
is)
{code:title=RecoveryService}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Queueing Non-immediate Summary/Recovery event of type"
- + eventType.name());
- }
+ LOG.debug(
+ "Queueing Non-immediate Summary/Recovery event of type {}",
+ eventType);
{code}
> Review of Debug Logging
> -----------------------
>
> Key: TEZ-4095
> URL: https://issues.apache.org/jira/browse/TEZ-4095
> Project: Apache Tez
> Issue Type: Improvement
> Reporter: David Mollitor
> Priority: Minor
> Attachments: TEZ-4095.1.patch, TEZ-4095.2.patch, TEZ-4095.2.patch
>
>
> # Remove superfluous debug logging guards
> # Create parameterized logging statements where appropriate
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
