Skip to content
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.

Sign up for GitHub

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

Jump to bottom

HDFS-15594. Lazy calculate live datanodes in safe mode tip #2332

Merged
merged 5 commits into from Sep 25, 2020
Merged

HDFS-15594. Lazy calculate live datanodes in safe mode tip #2332

merged 5 commits into from Sep 25, 2020

Conversation

NickyYe
Copy link
Contributor

@NickyYe NickyYe commented Sep 23, 2020

https://issues.apache.org/jira/browse/HDFS-15594

NOTICE

Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute

Hexiaoqiao
Hexiaoqiao approved these changes Sep 24, 2020
View reviewed changes
Copy link
Contributor

@Hexiaoqiao Hexiaoqiao left a comment

Choose a reason for hiding this comment

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

Thanks @NickyYe for your works, +1 from my side.
cc @goiri would you mind to have another check?

goiri
goiri reviewed Sep 24, 2020
View reviewed changes
...p-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManagerSafeMode.java Outdated
msg += String.format("The number of live datanodes %d has reached "
+ "the minimum number %d. ",
numLive, datanodeThreshold);
msg += "The number of live datanodes is not calculated " +
Copy link
Member

Choose a reason for hiding this comment

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

As we are at it, does it make sense to use StringBuilder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@goiri
Copy link
Member

goiri commented Sep 24, 2020

Thanks @NickyYe for the StringBuilder, that will reduce some memory too.
I'm a little surprised that Yetus is not kicking in.
@ayushtkn are you familiar on how to kick it? And now that I have your attention... any issues you can see with changing the message.

ayushtkn
ayushtkn approved these changes Sep 25, 2020
View reviewed changes
Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

I have triggered the build -
https://ci-hadoop.apache.org/job/hadoop-multibranch/view/change-requests/job/PR-2332/4/

The improvement seems fair enough, I don't think the message change would create any problem, Changing messages at UI is allowed.
It would change the exception text, I guess when there is a SafeModeException through checkNameNodeSafeMode in one case, but I don't think that would bother anyone as such. Should be all good.

@goiri
Copy link
Member

goiri commented Sep 25, 2020

There were a lot of failures because TestFileChecksum:
https://ci-hadoop.apache.org/job/hadoop-multibranch/view/change-requests/job/PR-2332/4/testReport/
But they are not related.

@goiri goiri merged commit 00c4de6 into apache:trunk Sep 25, 2020
bilaharith pushed a commit to bilaharith/hadoop that referenced this pull request Sep 27, 2020
@NickyYe @bilaharith
HDFS-15594. Lazy calculate live datanodes in safe mode tip (apache#2332)
c225ad6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goiri goiri goiri approved these changes

@Hexiaoqiao Hexiaoqiao Hexiaoqiao approved these changes

@ayushtkn ayushtkn ayushtkn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@NickyYe @goiri @Hexiaoqiao @ayushtkn