Skip to content

[FLINK-38054] Fix OperationSerializer issue for es8 sink - #159

Merged
reswqa merged 1 commit into
apache:mainfrom
reswqa:FLINK-38054
Jul 8, 2026
Merged

[FLINK-38054] Fix OperationSerializer issue for es8 sink#159
reswqa merged 1 commit into
apache:mainfrom
reswqa:FLINK-38054

Conversation

@reswqa

@reswqa reswqa commented Jul 7, 2026

Copy link
Copy Markdown
Member

By default, OperationSerializer uses AppClassLoader as the class loader for Kryo. When the user ship the es connector uber jar into the job's jar (Loading via UserCodeClassLoader), they will encounter a ClassNotFound issue.

BTW, I created FLINK-40085 to add an end-to-end test for the es8 sink also.


This coauthored by @mayorandrew, the original pr is #136. I think using a custom serializer for JsonNode is a bit of a hack and it cannot cover all situations. This pr manage to address this issue in another way.

Co-authored-by: Andrey Starostin <andrey.starostin@choco.com>
Comment on lines +40 to +42
DefaultInstantiatorStrategy instantiatorStrategy = new DefaultInstantiatorStrategy();
instantiatorStrategy.setFallbackInstantiatorStrategy(new StdInstantiatorStrategy());
kryo.setInstantiatorStrategy(instantiatorStrategy);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

DefaultInstantiatorStrategy is the recommended way of creating objects with Kryo. It runs constructors just like would be done with Java code. StdInstantiatorStrategy uses JVM specific APIs to create an instance of a class without calling any constructor at all. Using this is dangerous because most classes expect their constructors to be called. Creating the object by bypassing its constructors may leave the object in an uninitialized or invalid state. (https://github.com/EsotericSoftware/kryo?tab=readme-ov-file#instantiatorstrategy)

This address #136 (comment)

@reswqa

reswqa commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

@mayorandrew, Thanks for reporting and fixing this 👍 , it's a critical bug. Since you haven't updated the original PR for a long time, I took over it to unblock the plan of release next version. Your copyright has already been retained through co-authorship.

@wenjin272 wenjin272 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking this on @reswqa. LGTM

@reswqa

reswqa commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Thanks for the review.

@reswqa
reswqa merged commit ad7464e into apache:main Jul 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants