[SPARK-37628][BUILD] Upgrade Netty from 4.1.68 to 4.1.72#34881
[SPARK-37628][BUILD] Upgrade Netty from 4.1.68 to 4.1.72#34881LuciferYang wants to merge 5 commits into
Conversation
|
Test build #146135 has finished for PR 34881 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Kubernetes integration test status failure |
|
Test build #146139 has finished for PR 34881 at commit
|
|
May I ask you to run a maven build as well? Just to be in the safe side... I am not 100% sure but as you changed the exclusion in SparkBuild.scala but not in a pom.xml we might find something here. |
| minlog/1.3.0//minlog-1.3.0.jar | ||
| netty-all/4.1.68.Final//netty-all-4.1.68.Final.jar | ||
| netty-all/4.1.71.Final//netty-all-4.1.71.Final.jar | ||
| netty-buffer/4.1.71.Final//netty-buffer-4.1.71.Final.jar |
There was a problem hiding this comment.
Whoa, this doesn't look right. netty-all is (probably?) a packaging of all these subcomponents.
Something changes in how it's declared. We might need to depend just on pieces of netty 4.1.71?
And agree, I think you need Maven exclusions to match SBT
There was a problem hiding this comment.
Whoa, this doesn't look right. netty-all is (probably?) a packaging of all these subcomponents.
Something changes in how it's declared. We might need to depend just on pieces of netty 4.1.71?
The change occurred after version 4.1.69, Netty#11732 netty-all should not re-package jars.
@srowen Do you mean that we need to introduce only the sub modules we really use? I think it's OK
But we originally introduced all Netty modules before 4.1.68 through use netty-all-4.1.68.Final.jar. If we introduce subsets instead, will it cause some user compatibility problems?
There was a problem hiding this comment.
Let me investigate if there is a better way to upgrade first
There was a problem hiding this comment.
Or we can give up the upgrade of this version. I found that there are many users in the Netty community are giving feedback on this problem and hope to provide netty-all-xx.Final.jar again for upgrade.
There was a problem hiding this comment.
@srowen I read the discussion of Netty#11732 netty-all should not re-package jars and the recent issues&prs in Netty, although many people have dissenting opinion to the current usage way, it seems that netty-all uber jar has not been restored until 4.1.72, and no one has plans to do this. Do we need to wait for a new version that provides uber jar before upgrading?
There was a problem hiding this comment.
We can take our time here and should not be in hurry because of the log4j fix went in to netty. As I see in netty log4j-core was used only in test:
$ rg -A2 log4j-core
common/pom.xml
78: <artifactId>log4j-core</artifactId>
79- <scope>test</scope>
80- </dependency>
pom.xml
706: <artifactId>log4j-core</artifactId>
707- <version>${log4j2.version}</version>
708- <scope>test</scope>
In SPARK-35134 (#32230), there is a netty version conflict issue during sbt test process, the phenomenon that triggers the bug is that even if we use So I add 2 new We didn't find this problem during Maven testing, so I didn't make corresponding change for Maven In SPARK-35134. Then after Netty-4.1.69 (Netty#11732 netty-all should not re-package jars) , netty-all replaces a fat jar with separate modules, keeping Therefore, after upgrading the Netty to 4.1.71, the |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146215 has finished for PR 34881 at commit
|
| metrics-jvm/4.2.2//metrics-jvm-4.2.2.jar | ||
| minlog/1.3.0//minlog-1.3.0.jar | ||
| netty-all/4.1.68.Final//netty-all-4.1.68.Final.jar | ||
| netty-all/4.1.72.Final//netty-all-4.1.72.Final.jar |
There was a problem hiding this comment.
I see, so netty-all no longer has the contents of all the JARs below, so it's not redundant? that is OK if so. We could even exclude probably lots of these (SMTP? SOCKS?). Not necessary but might be nice if they're largeish and unused.
BTW the description says this updates log4j, but, log4j 2 is not in the build before or after this
There was a problem hiding this comment.
OK, let me try to clean up unnecessary Netty submodules
BTW the description says this updates log4j, but, log4j 2 is not in the build before or after this
I have deleted the relevant description. As @attilapiros said, the upgrade of log4j2 in Netty does not affect Spark
There was a problem hiding this comment.
@srowen @attilapiros e34a2c9 I clean up 19 Netty sub-modules that seem not to be used by spark, waiting for CI to test. I will also test these locally with Maven.
The cleaned sub modules include:
netty-codec-dns/4.1.72.Final//netty-codec-dns-4.1.72.Final.jar
netty-codec-haproxy/4.1.72.Final//netty-codec-haproxy-4.1.72.Final.jar
netty-codec-http/4.1.72.Final//netty-codec-http-4.1.72.Final.jar
netty-codec-http2/4.1.72.Final//netty-codec-http2-4.1.72.Final.jar
netty-codec-memcache/4.1.72.Final//netty-codec-memcache-4.1.72.Final.jar
netty-codec-mqtt/4.1.72.Final//netty-codec-mqtt-4.1.72.Final.jar
netty-codec-redis/4.1.72.Final//netty-codec-redis-4.1.72.Final.jar
netty-codec-smtp/4.1.72.Final//netty-codec-smtp-4.1.72.Final.jar
netty-codec-socks/4.1.72.Final//netty-codec-socks-4.1.72.Final.jar
netty-codec-stomp/4.1.72.Final//netty-codec-stomp-4.1.72.Final.jar
netty-codec-xml/4.1.72.Final//netty-codec-xml-4.1.72.Final.jar
netty-handler-proxy/4.1.72.Final//netty-handler-proxy-4.1.72.Final.jar
netty-resolver-dns-classes-macos/4.1.72.Final//netty-resolver-dns-classes-macos-4.1.72.Final.jar
netty-resolver-dns-native-macos/4.1.72.Final/osx-aarch_64/netty-resolver-dns-native-macos-4.1.72.Final-osx-aarch_64.jar
netty-resolver-dns-native-macos/4.1.72.Final/osx-x86_64/netty-resolver-dns-native-macos-4.1.72.Final-osx-x86_64.jar
netty-resolver-dns/4.1.72.Final//netty-resolver-dns-4.1.72.Final.jar
netty-transport-rxtx/4.1.72.Final//netty-transport-rxtx-4.1.72.Final.jar
netty-transport-sctp/4.1.72.Final//netty-transport-sctp-4.1.72.Final.jar
netty-transport-udt/4.1.72.Final//netty-transport-udt-4.1.72.Final.jar
Sub modules that may be used include:
netty-all/4.1.72.Final//netty-all-4.1.72.Final.jar
netty-buffer/4.1.72.Final//netty-buffer-4.1.72.Final.jar
netty-codec/4.1.72.Final//netty-codec-4.1.72.Final.jar
netty-common/4.1.72.Final//netty-common-4.1.72.Final.jar
netty-handler/4.1.72.Final//netty-handler-4.1.72.Final.jar
netty-resolver/4.1.72.Final//netty-resolver-4.1.72.Final.jar
netty-tcnative-classes/2.0.46.Final//netty-tcnative-classes-2.0.46.Final.jar
netty-transport-classes-epoll/4.1.72.Final//netty-transport-classes-epoll-4.1.72.Final.jar
netty-transport-classes-kqueue/4.1.72.Final//netty-transport-classes-kqueue-4.1.72.Final.jar
netty-transport-native-epoll/4.1.72.Final/linux-aarch_64/netty-transport-native-epoll-4.1.72.Final-linux-aarch_64.jar
netty-transport-native-epoll/4.1.72.Final/linux-x86_64/netty-transport-native-epoll-4.1.72.Final-linux-x86_64.jar
netty-transport-native-kqueue/4.1.72.Final/osx-aarch_64/netty-transport-native-kqueue-4.1.72.Final-osx-aarch_64.jar
netty-transport-native-kqueue/4.1.72.Final/osx-x86_64/netty-transport-native-kqueue-4.1.72.Final-osx-x86_64.jar
netty-transport-native-unix-common/4.1.72.Final//netty-transport-native-unix-common-4.1.72.Final.jar
netty-transport/4.1.72.Final//netty-transport-4.1.72.Final.jar
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146259 has finished for PR 34881 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146419 has finished for PR 34881 at commit
|
|
Merged to master |
### What changes were proposed in this pull request? Bump Netty version from 4.1.118.Final to 4.2.10.Final, which follows the official community migration guide: [Netty-4.2-Migration-Guide](https://github.com/netty/netty/wiki/Netty-4.2-Migration-Guide). ### Why are the changes needed? The Netty 4.2.10.Final version has been released, which netty version is 4.1.118.Final at present. Backport: - apache/spark#34881 - apache/spark#52552 - apache/spark#53499 ### Does this PR resolve a correctness bug? No. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? CI. Closes #3596 from SteNicholas/CELEBORN-2258. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: SteNicholas <programgeek@163.com>
What changes were proposed in this pull request?
This PR upgrades Netty from 4.1.68 to 4.1.71.
All the changes from 4.1.68 to 4.1.71 are as follows:
And after Netty#11732, netty-all replaces a fat jar with separate modules, so this pr manually excludes some Netty sub-modules that Spark does not rely on.
After this pr, Spark will rely on the following Netty sub modules:
and the following Netty sub modules are not dependent on Spark :
Why are the changes needed?
Avoid some potential bugs by upgrading Netty to 4.1.72
Does this PR introduce any user-facing change?
No
How was this patch tested?
Pass the Jenkins or GitHub Action