-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-28702 TestBackupMerge fails 100% of times on flaky dashboard #6078
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
This PR changes what we want to test here... @bbeaudreault I think this is a very critical problem which should be considered in the backup&restore code. After HBASE-28625 gets in, if we copy snapshot between different file systems without specifying the ignore checksum flag, the copy will fail because data checksum highly depend on the file sytem implementation so the data checksum between different file systems can not be compared... We should have a way to deal with this, at least we should give user the ability to specify this flag if they want to backup to a different file system like S3... |
I'm not sure I follow @Apache9. Are you saying this PR should fix production code instead of test code? |
Yes, exactly. The test is for testing copying between different file systems, but the fix here just make it copy to the same file system... |
@Apache9 @bbeaudreault I get it, this PR is not just about fixing the test code, should fix the production code completely. And I will try a new commit to address this. |
For me I think the current PR can work at least, @bbeaudreault do you think this is an acceptable way to fix the checksum mismatch problem? Thanks. |
@Apache9 @bbeaudreault I'm not very familiar with the hbase-backup module. Please help review it ? Thanks. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
I suppose the addition of the flag can work.
Should we improve the comments and/or error message to make it clear that checksum may not match across filesystems? Otherwise user may not know how to handle the failure.
@bbeaudreault Thanks for your review. I have revised it. Could you take a look? Thanks. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
ping @bbeaudreault @Apache9 |
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 for fixing
…6078) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Bryan Beaudreault <[email protected]>
…pache#6078) Signed-off-by: Duo Zhang <[email protected]> Signed-off-by: Bryan Beaudreault <[email protected]>
https://issues.apache.org/jira/browse/HBASE-28702