subject:"\[GitHub\] \[hudi\] danny0405 commented on a diff in pull request #9327\: \[HUDI\-6617\] make HoodieRecordDelegate implement KryoSerializable"

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

2023-08-02 Thread via GitHub



danny0405 commented on code in PR #9327:
URL: https://github.com/apache/hudi/pull/9327#discussion_r1282541082


##
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieRecordDelegate.java:
##
@@ -70,4 +78,24 @@ public void testKryoSerializeDeserialize() {
 assertEquals(new HoodieRecordLocation("001", "file01"), 
hoodieRecordDelegate.getCurrentLocation().get());
 assertEquals(new HoodieRecordLocation("001", "file-01"), 
hoodieRecordDelegate.getNewLocation().get());
   }
+
+  public Kryo getKryoInstance() {
+final Kryo kryo = new Kryo();
+// This instance of Kryo should not require prior registration of classes
+kryo.setRegistrationRequired(false);
+kryo.setInstantiatorStrategy(new Kryo.DefaultInstantiatorStrategy(new 
StdInstantiatorStrategy()));
+// Handle cases where we may have an odd classloader setup like with 
libjars
+// for hadoop
+kryo.setClassLoader(Thread.currentThread().getContextClassLoader());
+
+// Register Hudi's classes
+new HoodieCommonKryoRegistrar().registerClasses(kryo);
+
+// Register serializers
+kryo.register(Utf8.class, new SerializationUtils.AvroUtf8Serializer());
+kryo.register(GenericData.Fixed.class, new GenericAvroSerializer<>());

Review Comment:
   Do we need a registration for avro classes?



-- 
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: commits-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

2023-08-02 Thread via GitHub



danny0405 commented on code in PR #9327:
URL: https://github.com/apache/hudi/pull/9327#discussion_r1281585998


##
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieRecordDelegate.java:
##
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.model;
+
+import org.apache.hudi.common.testutils.AvroBinaryTestPayload;
+import org.apache.hudi.common.testutils.SchemaTestUtil;
+import org.apache.hudi.common.util.Option;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.IndexedRecord;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TestHoodieRecordDelegate {
+  private HoodieRecordDelegate hoodieRecordDelegate;
+
+  @BeforeEach
+  public void setUp() throws Exception {
+SchemaTestUtil testUtil = new SchemaTestUtil();
+final List indexedRecords = 
testUtil.generateHoodieTestRecords(0, 1);
+final List hoodieRecords =
+indexedRecords.stream().map(r -> new HoodieAvroRecord(new 
HoodieKey("001", "/00/00"),
+new AvroBinaryTestPayload(Option.of((GenericRecord) 
r.collect(Collectors.toList());
+HoodieRecord record = hoodieRecords.get(0);
+record.setCurrentLocation(new HoodieRecordLocation("001", "file01"));
+record.setNewLocation(new HoodieRecordLocation("001", "file-01"));
+hoodieRecordDelegate = HoodieRecordDelegate.fromHoodieRecord(record);
+  }
+
+  @Test
+  public void testKryoSerializeDeserialize() {
+Kryo kryo = new Kryo();
+ByteArrayOutputStream baos = new ByteArrayOutputStream(1024);
+Output output = new Output(baos);
+hoodieRecordDelegate.write(kryo, output);

Review Comment:
   I'm not very familiar with Kryo, do we need to register explicitly the class 
for it?



-- 
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: commits-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

2023-08-01 Thread via GitHub



danny0405 commented on code in PR #9327:
URL: https://github.com/apache/hudi/pull/9327#discussion_r1281405234


##
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieRecordDelegate.java:
##
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.model;
+
+import org.apache.hudi.common.testutils.AvroBinaryTestPayload;
+import org.apache.hudi.common.testutils.SchemaTestUtil;
+import org.apache.hudi.common.util.Option;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import com.esotericsoftware.kryo.serializers.JavaSerializer;
+import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.IndexedRecord;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TestHoodieRecordDelegate {
+  private HoodieRecordDelegate hoodieRecordDelegate;
+  private static final Kryo kryo = new Kryo();
+
+  @BeforeEach
+  public void setUp() throws Exception {
+SchemaTestUtil testUtil = new SchemaTestUtil();
+final List indexedRecords = 
testUtil.generateHoodieTestRecords(0, 1);
+final List hoodieRecords =
+indexedRecords.stream().map(r -> new HoodieAvroRecord(new 
HoodieKey("001", "/00/00"),
+new AvroBinaryTestPayload(Option.of((GenericRecord) 
r.collect(Collectors.toList());
+HoodieRecord record = hoodieRecords.get(0);
+record.setCurrentLocation(new HoodieRecordLocation("001", "file01"));
+record.setNewLocation(new HoodieRecordLocation("001", "file-01"));
+hoodieRecordDelegate = HoodieRecordDelegate.fromHoodieRecord(record);
+
+kryo.register(HoodieRecordDelegate.class, new JavaSerializer());
+  }
+
+  @Test
+  public void TestSerializeDeserialize() {

Review Comment:
   ```suggestion
 public void testKryoSerializeDeserialize() {
   ```



-- 
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: commits-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

2023-08-01 Thread via GitHub



danny0405 commented on code in PR #9327:
URL: https://github.com/apache/hudi/pull/9327#discussion_r1281404983


##
hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieRecordDelegate.java:
##
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.model;
+
+import org.apache.hudi.common.testutils.AvroBinaryTestPayload;
+import org.apache.hudi.common.testutils.SchemaTestUtil;
+import org.apache.hudi.common.util.Option;
+
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import com.esotericsoftware.kryo.serializers.JavaSerializer;
+import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.IndexedRecord;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TestHoodieRecordDelegate {
+  private HoodieRecordDelegate hoodieRecordDelegate;
+  private static final Kryo kryo = new Kryo();
+
+  @BeforeEach
+  public void setUp() throws Exception {
+SchemaTestUtil testUtil = new SchemaTestUtil();
+final List indexedRecords = 
testUtil.generateHoodieTestRecords(0, 1);
+final List hoodieRecords =
+indexedRecords.stream().map(r -> new HoodieAvroRecord(new 
HoodieKey("001", "/00/00"),
+new AvroBinaryTestPayload(Option.of((GenericRecord) 
r.collect(Collectors.toList());
+HoodieRecord record = hoodieRecords.get(0);
+record.setCurrentLocation(new HoodieRecordLocation("001", "file01"));
+record.setNewLocation(new HoodieRecordLocation("001", "file-01"));
+hoodieRecordDelegate = HoodieRecordDelegate.fromHoodieRecord(record);
+
+kryo.register(HoodieRecordDelegate.class, new JavaSerializer());

Review Comment:
   What happens if we do not register the class in Kryo, does it sill use the 
kryo ser/deser ?



-- 
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: commits-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

2023-08-01 Thread via GitHub



danny0405 commented on code in PR #9327:
URL: https://github.com/apache/hudi/pull/9327#discussion_r1280310875


##
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecordDelegate.java:
##
@@ -122,4 +127,20 @@ public String toString() {
 + ", newLocation=" + newLocation
 + '}';
   }
+
+  @Override
+  public final void write(Kryo kryo, Output output) {
+kryo.writeObjectOrNull(output, hoodieKey, HoodieKey.class);
+kryo.writeClassAndObject(output, currentLocation.isPresent() ? 
currentLocation.get() : null);
+kryo.writeClassAndObject(output, newLocation.isPresent() ? 
newLocation.get() : null);
+  }
+
+  @Override
+  public final void read(Kryo kryo, Input input) {
+this.hoodieKey = kryo.readObjectOrNull(input, HoodieKey.class);
+HoodieRecordLocation newrl = (HoodieRecordLocation) 
kryo.readClassAndObject(input);
+this.currentLocation = newrl == null ? Option.empty() : Option.of(newrl);
+HoodieRecordLocation oldrl = (HoodieRecordLocation) 
kryo.readClassAndObject(input);
+this.newLocation = oldrl == null ? Option.empty() : Option.of(oldrl);
+  }

Review Comment:
   Option.ofNullable ? Can we write some tests for it.



-- 
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: commits-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

[GitHub] [hudi] danny0405 commented on a diff in pull request #9327: [HUDI-6617] make HoodieRecordDelegate implement KryoSerializable

5 matches

Site Navigation

Mail list logo

Footer information