-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDFS-17431. Fix log format for BlockRecoveryWorker#recoverBlocks #6643
Conversation
LGTM |
💔 -1 overall
This message was automatically generated. |
LOG.warn("recover Block: {} FAILED: {}", b, e); | ||
LOG.warn("recover Block: {} FAILED: ", b, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats wrong here? the number of placeholders are correct only, for the second one it will invoke e.toString(), now you are changing it to print the entire trace.
I don't think it is broken, it looks like it was intentional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haiyang1987 Could you pls elaborate about the issue with the existing log format?
Thanks.
Thanks @wzk784533 @ayushtkn @dineshchitlangia for your review. I found that the log format showed some problems, such as the mentioned in this issue #6635.
Hi sir @ayushtkn @dineshchitlangia @wzk784533 what dou you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM, thanks for elaborating on the issue and providing the fix.
Thanks @dineshchitlangia @ayushtkn @wzk784533 for your review and merge~ |
Description of PR
https://issues.apache.org/jira/browse/HDFS-17431
Fix log format for BlockRecoveryWorker#recoverBlocks