Skip to content

[HUDI-1200] Fix NullPointerException, CustomKeyGenerator does not work #1984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
liujinhui1994 wants to merge 1 commit into from
Closed

[HUDI-1200] Fix NullPointerException, CustomKeyGenerator does not work #1984

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hudi-spark/src/main/java/org/apache/hudi/keygen/KeyGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public abstract class KeyGenerator implements Serializable, KeyGeneratorInterfac
private static final String STRUCT_NAME = "hoodieRowTopLevelField";
private static final String NAMESPACE = "hoodieRow";

protected transient TypedProperties config;
protected TypedProperties config;
Copy link
Contributor

@bhasudha bhasudha Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended to be transient so that we don't serialize config in KeyGenerator s. Lets keep it that way. Since the issue is specifically with CustomKeyGenerator we should resolve there.

Copy link
Contributor Author

@liujinhui1994 liujinhui1994 Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you let me know what you think? Can you describe it in detail @bhasudha

Copy link
Contributor

@wangxianghu wangxianghu Aug 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended to be transient so that we don't serialize config in KeyGenerator s. Lets keep it that way. Since the issue is specifically with CustomKeyGenerator we should resolve there.

Hi @bhasudha , CustomKeyGenerator extends KeyGenerator, maybe this NPE is caused by the config(erased in the process of serialization) in KeyGenerator. @liujinhui1994 is that what you mean?

Copy link
Contributor

@bhasudha bhasudha Aug 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so too. @liujinhui1994 can you try with Hudi 0.6.0 and see if that helps? We fixed some serialization issues there wrt KeyGenerators.

Copy link
Contributor

@wangxianghu wangxianghu Aug 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so too. @liujinhui1994 can you try with Hudi 0.6.0 and see if that helps? We fixed some serialization issues there wrt KeyGenerators.

I tried both TimestampBasedKeyGenerator and CustomKeyGenerator with 0.6.0 (in my IDEA) TimestampBasedKeyGenerator is ok, while CustomKeyGenerator still has NPE.
Remove transient can resolve this problem.

Caused by: java.lang.NullPointerException at org.apache.hudi.keygen.SimpleKeyGenerator.<init>(SimpleKeyGenerator.java:35) at org.apache.hudi.keygen.CustomKeyGenerator.getRecordKey(CustomKeyGenerator.java:128) at org.apache.hudi.keygen.BuiltinKeyGenerator.getKey(BuiltinKeyGenerator.java:75) at org.apache.hudi.HoodieSparkSqlWriter$$anonfun$1.apply(HoodieSparkSqlWriter.scala:143) at org.apache.hudi.HoodieSparkSqlWriter$$anonfun$1.apply(HoodieSparkSqlWriter.scala:139) at scala.collection.Iterator$$anon$11.next(Iterator.scala:410)

Copy link
Contributor

@pratyakshsharma pratyakshsharma Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhasudha @vinothchandar what was the purpose behind keeping this config object here as transient? I see it has been like this from the very beginning. There is a workaround solution also which only affects CustomKeyGenerator class but removing transient keyword from here actually seems the easier solution. :)

Copy link
Contributor

@pratyakshsharma pratyakshsharma Sep 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do not want to make config object as transient, we have 2 choices -

  1. introduce a new TypedProperties instance in CustomKeyGenerator
  2. introduce a new TimestampBasedKeyGenerator instance in CustomKeyGenerator, which gets initialised only if needed.

Please suggest.

private transient Function1<Object, Object> converterFn = null;

protected KeyGenerator(TypedProperties config) {
Expand Down