[HUDI-6617] make HoodieRecordDelegate implement KryoSerializable#9327
[HUDI-6617] make HoodieRecordDelegate implement KryoSerializable#9327danny0405 merged 6 commits intoapache:masterfrom
Conversation
| this.currentLocation = newrl == null ? Option.empty() : Option.of(newrl); | ||
| HoodieRecordLocation oldrl = (HoodieRecordLocation) kryo.readClassAndObject(input); | ||
| this.newLocation = oldrl == null ? Option.empty() : Option.of(oldrl); | ||
| } |
There was a problem hiding this comment.
Option.ofNullable ? Can we write some tests for it.
| record.setNewLocation(new HoodieRecordLocation("001", "file-01")); | ||
| hoodieRecordDelegate = HoodieRecordDelegate.fromHoodieRecord(record); | ||
|
|
||
| kryo.register(HoodieRecordDelegate.class, new JavaSerializer()); |
There was a problem hiding this comment.
What happens if we do not register the class in Kryo, does it sill use the kryo ser/deser ?
| } | ||
|
|
||
| @Test | ||
| public void TestSerializeDeserialize() { |
There was a problem hiding this comment.
| public void TestSerializeDeserialize() { | |
| public void testKryoSerializeDeserialize() { |
| Kryo kryo = new Kryo(); | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(1024); | ||
| Output output = new Output(baos); | ||
| hoodieRecordDelegate.write(kryo, output); |
There was a problem hiding this comment.
I'm not very familiar with Kryo, do we need to register explicitly the class for it?
There was a problem hiding this comment.
Thank you for your reminding.
Yes, it's necessary to register hudi specific classes. HoodieRecordDelegate has been added to HoodieCommonKryoRegistrar, that make sure the ordering of the registration could not change.
|
|
||
| // Register serializers | ||
| kryo.register(Utf8.class, new SerializationUtils.AvroUtf8Serializer()); | ||
| kryo.register(GenericData.Fixed.class, new GenericAvroSerializer<>()); |
There was a problem hiding this comment.
Do we need a registration for avro classes?
There was a problem hiding this comment.
No, the member variable types of HoodieRecordDelegate don't contain avro.that should't be registered.
Has been updated.
|
@prashantwason can you review |
|
@prashantwason I'm gonna merge it first, maybe you can revise it before the release. |
4 participants
Change Logs
make
HoodieRecordDelegateimplementKryoSerializableImpact
Improve serialize/deserialize performance.
Risk level (write none, low medium or high below)
NONE
Documentation Update
NONE
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist