-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16658. Change logLevel from DEBUG to INFO if logEveryBlock is true #4559
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
| (node.isDecommissioned() || node.isDecommissionInProgress()) ? 0 : 1; | ||
| if (logEveryBlock) { | ||
| blockLog.debug("BLOCK* addStoredBlock: {} is added to {} (size={})", | ||
| blockLog.info("BLOCK* addStoredBlock: {} is added to {} (size={})", |
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. Please double check if it could flood general log in some corner case.
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.
Thanks @Hexiaoqiao for your review. We have released it in our prod environment and it works well.
logEveryBlock=true means that we should output some logs about this block. And I have checked the callers and they are expected.
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.
LGTM. +1 from my side.
|
@ZanderXu Thank you for your contribution, pr looks good, but I want to know if it can be solved using dynamic debug log? I remember that hadoop provides a switch that can directly open the debug log. This command can temporarily modify the log level. If you directly modify the log level, will the log size be very large, after all, this is a block-level log? |
|
Thanks @slfan1989 for your review.
This command can change the log level of the logger. But it will print all debug log of the logger.
The log size is controllable because Maybe we should think about its use. This log is very helpful for us to locate some abnormal case about replica of block, such as complete failure, missing block, etc... |
Thanks for your explanation, I understand your changes. |
|
Committed to trunk. Thanks @ZanderXu for your contributions. Thanks @slfan1989 for your comments. |
|
Hi, @ZanderXu. |
…ue (apache#4559). Contributed by ZanderXu. Signed-off-by: He Xiaoqiao <[email protected]>
Description of PR
During locating some abnormal cases about block replication in our prod environment, I found that BlockManager does not out put some logs in
addStoredBlockeven thoughlogEveryBlockis true.I feel that we need to change the log level from DEBUG to INFO when
logEveryBlockis true. So that we can more easily locate some abnormal cases.