-
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-28460 Full backup restore failed on empty HFiles #5782
HBASE-28460 Full backup restore failed on empty HFiles #5782
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +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.
While you are here, can you fix the LOG name in RestoreTool to match the classname? Right now it's set as BackupUtils which is misleading.
As a big aside, I wonder if we should simplify FULL restore to just use the existing region boundaries in the snapshot manifest
hbase-backup/src/test/java/org/apache/hadoop/hbase/backup/TestFullRestore.java
Outdated
Show resolved
Hide resolved
throw new IOException("Can not restore from backup directory " + dirs | ||
+ " (check Hadoop and HBase logs). Bulk loader returns null"); | ||
} | ||
loader.bulkLoad(newTableNames[i], bulkOutputPath); |
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.
Is this a safe change? It solves for 1 case where empty hfiles could fail the restore, but I wonder if there are other cases where the bulkload might unexpectedly not restore any files.
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 don't have enough knowledge about the internals to give an informed opinion I'm afraid. The test suite finds no problem with the change, but that's no guarantee of course.
But I think that if there would be cases where not loading anything in the bulkload indicated an error, it would have to be detected at other locations rather than here. Restoring no files is a valid usecase, and so the only thing I can think of is that there would be an issue when those HFiles are written, or that something happens to those files before they are restored.
- The former should already be detected as an issue I'd hope.
- The latter could be something that damages or deletes the HFiles. The only (valid) to detect that would be to track how many (or even which specific HFiles) should be loaded. So metadata that would be recorded when creating the backup, and verified when restoring. But - the assumption that this could happen, would have a lot more repercussions outside of backup&restore in my opinion.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
…les (apache#5782) Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
No description provided.