-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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.
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
[FLINK-32817] Harnessing Jackson for Secure Serialization of YarnLocalResourceDescriptor #23292
Conversation
…lResourceDescriptor
Hello @xintongsong , if you have a moment, could you kindly review this PR? Thank you. |
@flinkbot run azure |
@YesOrNo828, please take a look into the CI failures. The failures are all from the Yarn deployment, which are likely related to the changes of this PR. |
@xintongsong Thanks for your confirmation, I'm working on that. |
Yarn nodemanager exports the environment variables that are inconsistent with the submitted string, the double-quotes are removed after exporting environment variables, so e.g.:
I found out Jackson allow to configure custom quote character to serialize Object, for example: single-quote, later I'll figure out the exactly way to serialize the string. |
@flinkbot run azure |
@xintongsong The CI is a success now. Do you have time to take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @YesOrNo828. LGTM. Merging.
What is the purpose of the change
If you try to submit a flink jar to a yarn cluster with a filename that contains spaces, the task won't be able to parse the file path in
YarnLocalResourceDescriptor
. This will result in warning logs being printed by TaskExecutor repeatedly.To avoid this issue, I plan to use Jackson for serializing and deserializing the
YarnLocalResourceDescriptor
instance, which will make the process safer.Brief change log
YarnLocalResourceDescriptor
YarnLocalResourceDescriptor
into a JSON string, and utilize the ObjectMapper for serialization and deserialization.Verifying this change
This change added tests and can be verified as follows:
YarnLocalResourceDescriptionTest
about the file name containing a spaceDoes this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation