Skip to content
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-33331] Update netty to 4.1.100 and arrow to 13.0.0 to make flink-python passing on java 21 #23664

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

snuyanzin
Copy link
Contributor

What is the purpose of the change

To support java 21.
Since JDK21 there was removed of constructor within https://bugs.openjdk.org/browse/JDK-8303083
Netty should support it and the support was added within netty/netty#13366 which is a part of 4.1.93.Final. Also need to bump arraw as well

Verifying this change

This change is a trivial rework

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes )
  • The public API, i.e., is any changed class annotated with @Public(Evolving): ( no)
  • The serializers: (no )
  • The runtime per-record code paths (performance sensitive): (no )
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no )
  • The S3 file system connector: (no )

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@flinkbot
Copy link
Collaborator

flinkbot commented Nov 5, 2023

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@TanYuxin-tyx TanYuxin-tyx left a comment

Choose a reason for hiding this comment

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

@snuyanzin Thanks for updating the version. I have a minor question about the PR. PTAL, thanks.

@@ -887,7 +887,7 @@ under the License.
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-bom</artifactId>
<version>4.1.70.Final</version>
<version>4.1.100.Final</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @snuyanzin Thank you for the netty version update. I have been working with netty recently and have noticed a hardcoded netty version in the flink-rpc-akka/pom.xml (

<version>3.10.6.Final</version>
). Do we need to update that version?
In addition, it appears that the original hardcoded version might not be accurate.

Copy link
Contributor Author

@snuyanzin snuyanzin Nov 7, 2023

Choose a reason for hiding this comment

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

Hi @TanYuxin-tyx
thanks for the question
the reason why upgrade of netty and arrow is required is failing of number of python tests while building with jdk21

I have my own pipepline setup with jdk21
example of error could be seen e.g. https://dev.azure.com/snuyanzin/flink/_build/results?buildId=2490&view=logs&j=fba17979-6d2e-591d-72f1-97cf42797c11&t=727942b6-6137-54f7-1ef9-e66e706ea068

same error was mentioned in FLINK-33331 description.

Since both netty and arrow require this removed constructor during python test they both need bumping the version.
By the way here it is the result of same pipeline with cherry-picked commit from this PR https://dev.azure.com/snuyanzin/flink/_build/results?buildId=2491&view=results
it could be seen that after that python tests with java 21 are passing (there are some other failures for other tests which are being fixed within other jira issue under FLINK-33163)

About the dependency you've mentioned: this dependency is used only for Pekko (former Akka) since current Pekko version can not run with netty 4.x... At the same time based on tests it is not an issue, looks like they don't use this removed constructor... and no need to bump the version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more note about flink-rpc-akka dependency

in theory it could be update in future within a separate task
however first there should be support of Netty 4 from Pekko's side which is done within apache/pekko#643
however it is not released yet...

Copy link
Contributor

Choose a reason for hiding this comment

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

@snuyanzin OK, make sense. Thanks for your patient explanations.

@snuyanzin
Copy link
Contributor Author

Thanks for taking a look

@snuyanzin snuyanzin merged commit c488adf into apache:master Nov 12, 2023
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.

3 participants