DieterDP-ng commented on code in PR #6706:
URL: https://github.com/apache/hbase/pull/6706#discussion_r1983735489
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##########
@@ -165,20 +166,27 @@ protected List<BulkLoad> handleBulkLoad(List<TableName>
tablesToBackup) throws I
srcTableQualifier);
LOG.trace("copying {} to {}", p, tgt);
}
- activeFiles.add(p.toString());
+ bulkloadInfo.addActiveFile(p.toString());
} else if (fs.exists(archive)) {
LOG.debug("copying archive {} to {}", archive, tgt);
- archiveFiles.add(archive.toString());
+ bulkloadInfo.addArchiveFiles(archive.toString());
}
- mergeSplitBulkloads(activeFiles, archiveFiles, srcTable);
- incrementalCopyBulkloadHFiles(tgtFs, srcTable);
+ toBulkload.put(srcTable, bulkloadInfo);
}
+
+ for (MergeSplitBulkloadInfo bulkloadInfo : toBulkload.values()) {
+ mergeSplitBulkloads(bulkloadInfo);
+ incrementalCopyBulkloadHFiles(tgtFs, bulkloadInfo.getSrcTable());
+ }
+
return bulkLoads;
}
- private void mergeSplitBulkloads(List<String> activeFiles, List<String>
archiveFiles,
- TableName tn) throws IOException {
+ private void mergeSplitBulkloads(MergeSplitBulkloadInfo bulkload) throws
IOException {
Review Comment:
Using the wrapper class here sort of obfuscates what is happening. As in: I
now would have to check the details of the `MergeSplitBulkloadInfo` or contents
of this method.
I'd suggest to keep the parameters of this method unchanged, and instead
unwrap the values in the calling method.
Alternatively (though the former has my preference), find a more suited name
for the wrapper class, and add some javadoc to it.
##########
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java:
##########
@@ -165,20 +166,27 @@ protected List<BulkLoad> handleBulkLoad(List<TableName>
tablesToBackup) throws I
srcTableQualifier);
LOG.trace("copying {} to {}", p, tgt);
}
- activeFiles.add(p.toString());
+ bulkloadInfo.addActiveFile(p.toString());
} else if (fs.exists(archive)) {
LOG.debug("copying archive {} to {}", archive, tgt);
- archiveFiles.add(archive.toString());
+ bulkloadInfo.addArchiveFiles(archive.toString());
}
- mergeSplitBulkloads(activeFiles, archiveFiles, srcTable);
- incrementalCopyBulkloadHFiles(tgtFs, srcTable);
+ toBulkload.put(srcTable, bulkloadInfo);
Review Comment:
This line can be scrapped, the `computeIfAbsent` should have already added
it to the map.
--
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]
