You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "mateczagany (via GitHub)" <gi...@apache.org> on 2026/01/13 19:43:39 UTC
[PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
mateczagany opened a new pull request, #27407:
URL: https://github.com/apache/flink/pull/27407
## What is the purpose of the change
Update flink-shaded to 21.0, with the biggest change being the new Netty version.
## Brief change log
- Update flink-shaded to 21.0
- Change `security.ssl.verify-hostname` default to `false`, as it was never enabled before anyways, but the new default Netty behavior is to verify hostnames
- Remove redundant Netty version from `flink-kubernetes`
- Pin older Netty version to `flink-python`, as Apache Arrow can only work with very specific Netty versions.
- Update deprecated Netty calls
## Verifying this change
- Compiled locally and started example jobs
- Make sure that CI passes
## 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): yes
- 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2693284773
##########
flink-runtime-web/src/test/resources/rest_api_v1.snapshot:
##########
@@ -1549,7 +1469,7 @@
"properties" : {
"checkpointed_size" : {
"type" : "object",
- "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:util.stats:StatsSummaryDto",
+ "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:util:stats:StatsSummaryDto",
Review Comment:
This file is generated by `flink-shaded-jackson-module-jsonSchema`, and I had to re-generate it because `RuntimeRestAPIStabilityTest` was failing otherwise, instructing me to re-generate this file.
According to the test, the API was modified in a compatible way with this upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2692241017
##########
flink-runtime-web/src/test/resources/rest_api_v1.snapshot:
##########
@@ -1549,7 +1469,7 @@
"properties" : {
"checkpointed_size" : {
"type" : "object",
- "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:util.stats:StatsSummaryDto",
+ "id" : "urn:jsonschema:org:apache:flink:runtime:rest:messages:util:stats:StatsSummaryDto",
Review Comment:
why is this related to flink-shaded upgrade?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #27407:
URL: https://github.com/apache/flink/pull/27407#issuecomment-3751931277
based on trace from the recent failure https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=71641&view=logs&j=0da23115-68bb-5dcd-192c-bd4c8adebde1&t=1ffc5ec2-7913-50ff-0177-3fca16f1b8f0&l=78731
the error is related to changes in this PR
@mateczagany can you take a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #27407:
URL: https://github.com/apache/flink/pull/27407#issuecomment-3746253880
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "253648887cc7c444a888bd05ece82e9fb53668a5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "253648887cc7c444a888bd05ece82e9fb53668a5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 253648887cc7c444a888bd05ece82e9fb53668a5 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2689020818
##########
flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java:
##########
@@ -519,7 +519,7 @@ public static Configuration forProvider(Configuration configuration, String prov
public static final ConfigOption<Boolean> SSL_VERIFY_HOSTNAME =
key("security.ssl.verify-hostname")
.booleanType()
- .defaultValue(true)
+ .defaultValue(false)
Review Comment:
I should have added more information in the description of the PR, so I'll try to explain my motivation here:
This configuration was not used before anywhere for a long time, and Flink only relied on the default behavior of Netty. This default behavior in 4.1.x was to not verify the hostnames. Since 4.2.x, as in the [migration guide](https://github.com/netty/netty/wiki/Netty-4.2-Migration-Guide#compatibility-highlights), Netty now enables hostname verification by default.
So I think this is the best way to ensure that there are no surprises for people that upgrade.
But I will create another PR for the Netty changes as you've advised, and we can discuss it further there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2694777002
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
Oh, nevermind, this is the same issue as before, I was on wrong branch locally. Unfortunately keeping the Netty version as-is did not help get rid of this.
If you try to run this test suite locally, you will see these errors: `Received fatal alert: certificate_unknown`. This is because starting from Netty 4.2, SSL hostname verification is on by default. Setting `security.ssl.verify-hostname` has NO effect, because this is not used anywhere. So I suggest to change its default value to `false`, and use it to set SSL hostname verification, as that would allow us to have the same behavior as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2691533707
##########
pom.xml:
##########
@@ -914,7 +914,7 @@ under the License.
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-bom</artifactId>
- <version>4.1.100.Final</version>
+ <version>4.2.6.Final</version>
Review Comment:
@snuyanzin thank you for your reviews, I will create other PRs after this gets merged:
- Upgrade Netty
- Upgrade Pekko
I also experienced a few test failures after upgrading flink-shaded-netty, but I couldn't find out exactly what causes these. What I know is that they are all intermittent failures, caused by various race conditions. My suspicion is that some Netty internals have changed, as all these newly failing tests relied on subtle behavior and timing between components.
I don't think we'll have a successful CI like this, but I'll create PRs to fix each flaky tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2689021828
##########
pom.xml:
##########
@@ -914,7 +914,7 @@ under the License.
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-bom</artifactId>
- <version>4.1.100.Final</version>
+ <version>4.2.6.Final</version>
Review Comment:
Thank you, I think that's a great idea. I'll shave off this PR to only contain the flink-shaded upgrade, and create another PR for Netty-specific changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #27407:
URL: https://github.com/apache/flink/pull/27407
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2687876244
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
The response should be written from Netty's event loop thread, which is enforced by Netty if called from another thread.
However, when enforced by Netty, sometimes the buffer won't be fully written if the server closes right after writing the buffer, triggering intermittent failures in `RestServerEndpointITCase#testShouldWaitForHandlersWhenClosing`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2694777002
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
Oh, nevermind, this is the same issue as before, I was on wrong branch locally. Unfortunately keeping the Netty version as-is did not help get rid of this.
If you try to run this test suite locally, you will see these errors: `Received fatal alert: certificate_unknown`. This is because starting from Netty 4.2, SSL hostname verification is on by default. Setting `security.ssl.verify-hostname` has NO effect, because this is not used anywhere. So I suggest to change its default value to `false`, and use it to set SSL hostname verification, as that would allow us to have the same behavior as before.
The reason you don't see any errors in the CI is that this error causes one of the tests to hang indefinitely, so no output will be shown of the errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2693277077
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
@snuyanzin the issue was this stuck thread:
```
Jan 14 21:15:14 at org.apache.flink.core.testutils.BlockerSync.awaitBlocker(BlockerSync.java:59)
Jan 14 21:15:14 - locked <0x00000000aa141c48> (a java.lang.Object)
Jan 14 21:15:14 at org.apache.flink.runtime.rest.RestServerEndpointITCase.testShouldWaitForHandlersWhenClosing(RestServerEndpointITCase.java:597)
```
Something has changed internally in Netty, and this test consistently times out on Azure CI. The issue arises when the response is not written from the event loop, and Netty enqueues the write operations on the event loop instead, and then we close the REST server itself (which is the point of the test), the writes don't always go through properly.
By running the write operations on the event loop, this issue goes away. I think it's better to do it this way anyways, so that Netty does not have to create 3 write tasks to eventually execute on the event loop thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "mateczagany (via GitHub)" <gi...@apache.org>.
mateczagany commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2693277077
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
<del>@snuyanzin the issue was this stuck thread:</del>
```
Jan 14 21:15:14 at org.apache.flink.core.testutils.BlockerSync.awaitBlocker(BlockerSync.java:59)
Jan 14 21:15:14 - locked <0x00000000aa141c48> (a java.lang.Object)
Jan 14 21:15:14 at org.apache.flink.runtime.rest.RestServerEndpointITCase.testShouldWaitForHandlersWhenClosing(RestServerEndpointITCase.java:597)
```
<del>Something has changed internally in Netty, and this test consistently times out on Azure CI. The issue arises when the response is not written from the event loop, and Netty enqueues the write operations on the event loop instead, and then we close the REST server itself (which is the point of the test), the writes don't always go through properly.</del>
<del>By running the write operations on the event loop, this issue goes away. I think it's better to do it this way anyways, so that Netty does not have to create 3 write tasks to eventually execute on the event loop thread.</del>
Nevermind, unfortunately this only solved the issue on my side own computer, but the CI still failed at the exact same steps. I'll continue digging...
##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/AbstractRestHandler.java:
##########
@@ -85,14 +85,15 @@ protected CompletableFuture<Void> respondToRequest(
response = FutureUtils.completedExceptionally(e);
}
- return response.thenAccept(
+ return response.thenAcceptAsync(
resp ->
HandlerUtils.sendResponse(
ctx,
httpRequest,
resp,
messageHeaders.getResponseStatusCode(),
- responseHeaders));
+ responseHeaders),
+ ctx.executor());
Review Comment:
<del>@snuyanzin the issue was this stuck thread:</del>
```
Jan 14 21:15:14 at org.apache.flink.core.testutils.BlockerSync.awaitBlocker(BlockerSync.java:59)
Jan 14 21:15:14 - locked <0x00000000aa141c48> (a java.lang.Object)
Jan 14 21:15:14 at org.apache.flink.runtime.rest.RestServerEndpointITCase.testShouldWaitForHandlersWhenClosing(RestServerEndpointITCase.java:597)
```
<del>Something has changed internally in Netty, and this test consistently times out on Azure CI. The issue arises when the response is not written from the event loop, and Netty enqueues the write operations on the event loop instead, and then we close the REST server itself (which is the point of the test), the writes don't always go through properly.</del>
<del>By running the write operations on the event loop, this issue goes away. I think it's better to do it this way anyways, so that Netty does not have to create 3 write tasks to eventually execute on the event loop thread.</del>
Nevermind, unfortunately this only solved the issue on my own computer, but the CI still failed at the exact same steps. I'll continue digging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2714764017
##########
flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java:
##########
@@ -519,7 +519,7 @@ public static Configuration forProvider(Configuration configuration, String prov
public static final ConfigOption<Boolean> SSL_VERIFY_HOSTNAME =
key("security.ssl.verify-hostname")
.booleanType()
- .defaultValue(true)
+ .defaultValue(false)
Review Comment:
yep searched in the code it is not used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2688157106
##########
flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java:
##########
@@ -519,7 +519,7 @@ public static Configuration forProvider(Configuration configuration, String prov
public static final ConfigOption<Boolean> SSL_VERIFY_HOSTNAME =
key("security.ssl.verify-hostname")
.booleanType()
- .defaultValue(true)
+ .defaultValue(false)
Review Comment:
Changing of the default value is very questionable step and shouldn't be done without justification and discussion in Mailing List imho
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2688157106
##########
flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java:
##########
@@ -519,7 +519,7 @@ public static Configuration forProvider(Configuration configuration, String prov
public static final ConfigOption<Boolean> SSL_VERIFY_HOSTNAME =
key("security.ssl.verify-hostname")
.booleanType()
- .defaultValue(true)
+ .defaultValue(false)
Review Comment:
Changing the default value is very questionable step and shouldn't be done without justification and discussion in Mailing List imho
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-38903] Update flink-shaded to 21.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #27407:
URL: https://github.com/apache/flink/pull/27407#discussion_r2688154395
##########
pom.xml:
##########
@@ -914,7 +914,7 @@ under the License.
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-bom</artifactId>
- <version>4.1.100.Final</version>
+ <version>4.2.6.Final</version>
Review Comment:
can we update flink-shaded and bump netty here in different PRs?
Probably it will simplify the review part
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org