DieterDP-ng commented on code in PR #5782:
URL: https://github.com/apache/hbase/pull/5782#discussion_r1547469740
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/mapreduce/MapReduceRestoreJob.java:
##########
@@ -87,10 +87,7 @@ public void run(Path[] dirPaths, TableName[] tableNames,
Path restoreRootDir,
LOG.debug("Restoring HFiles from directory " + bulkOutputPath);
}
- if (loader.bulkLoad(newTableNames[i], bulkOutputPath).isEmpty()) {
- throw new IOException("Can not restore from backup directory " +
dirs
- + " (check Hadoop and HBase logs). Bulk loader returns null");
- }
+ loader.bulkLoad(newTableNames[i], bulkOutputPath);
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
