Skip to content

TEZ-4095: Review of Debug Logging - #87

Merged
jteagles merged 2 commits into
apache:masterfrom
belugabehr:TEZ-4095
Jan 20, 2021
Merged

TEZ-4095: Review of Debug Logging#87
jteagles merged 2 commits into
apache:masterfrom
belugabehr:TEZ-4095

Conversation

@belugabehr

Copy link
Copy Markdown
Contributor

Trying this again, albeit as a GitHub PR and, for simplicity, I tried to only "fix" debug statements that do not require much modification

@abstractdogabstractdog left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, all of @jteagles's comments were addressed here, could you please confirm?

++numHeartbeats;
if (LOG.isDebugEnabled() || numHeartbeats % 50 == 1) {
LOG.info(constructPeriodicLog(freeResources));
LOG.debug(constructPeriodicLog(freeResources));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new change from the previous patch and a good catch that has been there since this file was created.

@jteagles
jteagles merged commit c2d48a6 into apache:masterJan 20, 2021
asfgit pushed a commit that referenced this pull request Jan 20, 2021
@belugabehr

Copy link
Copy Markdown
ContributorAuthor

@jteagles@abstractdog Thanks for the review (twice for @jteagles) and the merge!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@belugabehr@abstractdog@jteagles