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-16244.Add the necessary write lock in Checkpointer#doCheckpoint(). #3497
Conversation
💔 -1 overall
This message was automatically generated. |
@jojochuang @virajjasani , here are some questions I hope to get your help. In addition, there are some exceptions in jenkins, such as: |
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
But do people still use backup node? IMO, it is deprecated and should be removed in the future.
Thanks @jojochuang for the comments and review. |
@ayushtkn @ferhui @Hexiaoqiao, can you take some time to review this pr. |
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.
Makes sense, +1 (non-binding)
The failures in above tests are not relevant to this PR.
I doubt if this change can be unit tested, let's see what others recommend. |
Thanks @virajjasani for the comment and review. |
@ayushtkn @ferhui @Hexiaoqiao , are you willing to spend some time to help review this pr. |
@jianghuazhu Thanks for contribution. @virajjasani @jojochuang Thanks for review! Merged |
@ferhui Thank you very much. |
Description of PR
In Checkpointer#doCheckpoint(), when you need to execute reloadFromImageFile(), you need to add the necessary write lock.
Details: HDFS-16244
How was this patch tested?
Need to verify the correctness of Checkpointer#doCheckpoint().