bbeaudreault commented on code in PR #4770:
URL: https://github.com/apache/hbase/pull/4770#discussion_r1061515529
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat2.java:
##########
@@ -254,16 +255,19 @@ public void write(ImmutableBytesWritable row, V cell)
throws IOException {
byte[] tableNameBytes = null;
if (writeMultipleTables) {
tableNameBytes = MultiTableHFileOutputFormat.getTableName(row.get());
- tableNameBytes = TableName.valueOf(tableNameBytes).toBytes();
+ tableNameBytes =
TableName.valueOf(tableNameBytes).getNameWithNamespaceInclAsString()
Review Comment:
I reviewed the rest of this patch -- the modified classes not the net-new
ones (which i confirmed are identical to `master`). This change here (and the
related test changes) is the only concerning thing. I think once we fix this we
should be able to move forward with merging.
@apurtell to your question "is this change necessary?" -- Yes, _some_ change
is necessary due to a mismatch between the output path here and the path
expected in `HBackupFileSystem#getTableBackupDir`. I gave 2 options above to
make this backwards compatible, do you have any opinion on them?
I know we've discussed configurations in the past :) I'd like to avoid
unnecessary configurations generally, but a new one here (option 2) might be
best.
--
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]
