-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-16634. Dynamically adjust slow peer report size on JMX metrics #4448
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. |
| * the highest number of votes from peers. | ||
| */ | ||
| private final int maxNodesToReport; | ||
| private int maxNodesToReport; |
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.
Please set this to volatile. Although it doesn't make a big difference here, I think it's better to be consistent with other reconfig changes. What do you think of this?
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.
Yes, you have made good point @tomscut, this can be done to remain in line with other reconfig changes, however it might cause bit of a performance issue for JMX metrics API overall, hence I was bit reluctant to make the change. But if you have strong preference, I can make the change, WDYT?
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.
This field is almost always read-only and rarely changed, and for read-only operations, it doesn't have much impact. WDYT?
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.
Yeah it's fine I think, this is not a big concern either ways. So let me make this change.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
tomscut
left a comment
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.
|
Thanks @virajjasani for your contribution! |
|
Hi @virajjasani , could you please submit another PR for branch-3.3 since there are some conflicts when cherry-pick. Thanks. |
…pache#4448) Signed-off-by: Tao Li <tomscut@apache.org>
Description of PR
On a busy cluster, sometimes it takes bit of time for deleted node(from the cluster)'s "slow node report" to get removed from slow peer json report on Namenode JMX metrics. In the meantime, user should be able to browse through more entries in the report by adjusting i.e. reconfiguring "dfs.datanode.max.nodes.to.report" so that the list size can be adjusted without user having to bounce active Namenode just for this purpose.
How was this patch tested?
Dev cluster and using UT.
For code changes: