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-33417][netty] Upgrade 4.1.82 -> 4.1.83 #127

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

TanYuxin-tyx
Copy link

This is for https://issues.apache.org/jira/browse/FLINK-33417.

In our ARM environment, we encounter a compile error when using Flink 1.17.

Flink 1.17 depends on flink-shaded 16.1, which uses netty 4.1.82. However, flink-shaded 16.1 fails to compile in the ARM environment. As a result, we are unable to compile Flink 1.17 due to this issue.

We have tested compiling flink-shaded using netty 4.1.83 or a later version in ARM env, and it can compile successfully.

Taking into consideration the previous discussions regarding compatibility and the dependency of external connectors on this version, I propose addressing the bug by only updating flink-shaded's netty to a minor version (e.g., 4.1.83) rather than backporting FLINK-32032.

To implement the update, maybe a new release of flink-shaded 16.2 needs to be released.

The discussion details is at https://lists.apache.org/thread/y1c8545bcsx2836d9pgfdzj65knvw7kb.

@XComp
Copy link
Contributor

XComp commented Nov 2, 2023

thanks @TanYuxin-tyx - you don't have, by any chance, links to the referenced CI runs which you could share here as well?

Copy link
Contributor

@XComp XComp left a comment

Choose a reason for hiding this comment

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

The change itself looks good to me 👍

@TanYuxin-tyx
Copy link
Author

TanYuxin-tyx commented Nov 2, 2023

you don't have, by any chance, links to the referenced CI runs which you could share here as well?

@XComp Thanks a lot for the quick review.

The CI is here https://github.com/apache/flink-shaded/actions/runs/6727989844.
Because this is my first PR to this repo, the CI was not triggered automatically when opening it. After the first contribution, the CI will be triggered automatically.

@XComp
Copy link
Contributor

XComp commented Nov 2, 2023

Sorry for not being that clear. I meant the ARM compilation error you mentioned in the PR description:

In our ARM environment, we encounter a compile error when using Flink 1.17.

I was wondering whether there would be a link to the failure and a link to a run that succeeded with this fix being applied.

@TanYuxin-tyx
Copy link
Author

ok. I have only the screenshots of the error, unfortunately, I do not have any links to provide.

I have attached these screenshots to the Jira(https://issues.apache.org/jira/browse/FLINK-33417).
The BE20xxx pic is the error when using 4.1.82. The 09DDxx pic is the pic of compiling successfully after using 4.1.83.

Copy link
Member

@reswqa reswqa left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@XComp
Copy link
Contributor

XComp commented Nov 3, 2023

Thanks @reswqa for verifying the PR as well. There's a discussion ongoing in FLINK-33417. I'm gonna wait with merging this change until the last questions are answered.

@reswqa
Copy link
Member

reswqa commented Nov 3, 2023

I'm gonna wait with merging this change until the last questions are answered.

Yes, make sense.

@XComp XComp merged commit 2e81185 into apache:release-16.0 Nov 3, 2023
1 check passed
@XComp
Copy link
Contributor

XComp commented Nov 3, 2023

I merged the PR after a final investigation. The results of the investigation are collected in FLINK-33417. Thanks for your patience @TanYuxin-tyx

@TanYuxin-tyx
Copy link
Author

@XComp Thanks a lot for your investigation and the careful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants