offermannu commented on a change in pull request #3232:
URL: https://github.com/apache/hbase/pull/3232#discussion_r628175203
##########
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:
I tried this, but it doesn't work unfortunately. If you look at the
`readFields` method you'll see that it calls `KeyValue.create(DataInput in)`
which in turn calls [`KeyValue.create(int length, DataInput
in)`](https://github.com/apache/hbase/blob/6cfff27465620112eb308cf881e6321d928743ed/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java#L2281).
The length is read as integer from the DataInput which kind of matches what
`write` has written before in line 138 - so far so good (I'm refering to
original version btw).
Honestly I'm a little bit confused at this point and don't know whether
`length` expects the key length or the length of the whole key-value pair.
You'll see that `KeyValue` allocates a byte array of length `length`(line 2266)
and so I guess that it must be the length of key+value.
Now keep in mind that the first 4 bytes are consumed from DataInput (the
length) and `KeyValue.create` calls:
`in.readFully(bytes); return new KeyValue(bytes, 0, length);` - thus `bytes`
starts with four '0' bytes coming from `Import.write line 139:
out.writeInt(0);`!
The [KeyValue
constructor](https://github.com/apache/hbase/blob/6cfff27465620112eb308cf881e6321d928743ed/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java#L343)
in turn calls
[`KeyValueUtil.checkKeyValueBytes`](https://github.com/apache/hbase/blob/6cfff27465620112eb308cf881e6321d928743ed/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java#L520).
And here I discovered that the serialization can never match the
deserialization because `KeyValueUtil.checkKeyValueBytes` expects the key len
at the beginning of the `bytes`buffer (note that `offset==0`) but this '0' as
mentioned before.
To sum it up:
`CellWritableComparable.write`writes:
[ [4 bytes key length], [4 bytes '0'], [...] ]
`CellWritableComparable.readFields` expects:
[ [4bytes key-value length], [4 bytes key length], [...] ]
Honestly, I wonder how that ever worked ;-)
--
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:
[email protected]
