offermannu commented on a change in pull request #3232:
URL: https://github.com/apache/hbase/pull/3232#discussion_r626807264
##########
File path:
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
##########
@@ -135,9 +135,7 @@ public CellWritableComparable(Cell kv) {
@Override
public void write(DataOutput out) throws IOException {
- out.writeInt(PrivateCellUtil.estimatedSerializedSizeOfKey(kv));
- out.writeInt(0);
- PrivateCellUtil.writeFlatKey(kv, out);
+ KeyValueUtil.write(new KeyValue(kv), out);
Review comment:
Whatever `CellWritableComparable.write()`writes must be compatible with
`CellWritableComparable.readFields`
which is currently not the case (see [this exception in
HBASE-25839](https://issues.apache.org/jira/browse/HBASE-25839?focusedCommentId=17338869&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17338869).
The phrase "keyLength=0" inside the exception message comes actually from the
statement `out.writeIn(0)`in line 139).
My proposal aligns the "write" with the existing
`CellWritableComparable.readFields`. Theoretically one can adjust
`CellWritableComparable.readFields` so that it becomes compatible with the
current `write` method but this looks more complicated to me.
AFAICS the key is only used during the mapper sorting phase. The
[reducer](https://github.com/apache/hbase/blob/f76a601273e834267b55c0cda12474590283fd4c/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java#L267)
doesn't care about the key at all.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
