You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "xjlgod (via GitHub)" <gi...@apache.org> on 2026/08/22 12:09:33 UTC
[PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
xjlgod opened a new pull request, #8209:
URL: https://github.com/apache/incubator-seata/pull/8209
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<!-- Please make sure you have read and understood the contributing guidelines -->
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
- Upgrade Netty from `4.1.137.Final` to `4.2.15.Final` through the Netty BOM.
- Migrate Seata transport event loops from deprecated Netty 4.1 APIs to the Netty 4.2 `IoHandler` / `MultiThreadIoEventLoopGroup` APIs.
- Keep the allocator explicitly configured as `pooled`, and remove use of Netty internal APIs.
- Ensure Seata's Netty BOM takes precedence over Spring Boot dependency management in server-related modules.
- Align related gRPC/Protobuf dependency management and add required direct Netty dependencies for the SkyWalking plugin.
- Update the gRPC interceptor test to use an actual `grpc-netty` TCP server and client.
- Add Chinese and English change-log entries under `changes/`.
### Ⅱ. Does this pull request fix one issue?
No.
### Ⅲ. Why don't you add test cases (unit test/integration test)?
N/A. This PR updates existing tests and executes the relevant test suites.
In particular, `GrpcTest` was migrated from the in-process transport to a real `grpc-netty` TCP transport, so the test now verifies the Netty-based gRPC request path. Existing core remoting, HTTP/2, Etcd registry, and version-compatibility tests were also executed.
### Ⅳ. Describe how to verify it
1. Run the relevant unit tests on JDK 8:
- Core Netty remoting, HTTP/2 handler, channel event, and Netty client bootstrap tests.
- Etcd registry tests.
- `extensions/rpc/seata-grpc` interceptor tests.
2. Run the cross-version compatibility tests on JDK 21:
- Verify client/server combinations across the old and new versions.
3. Build the release distribution with JDK 25 and the `release-seata` profile.
4. Inspect the generated server and naming-server distributions:
- All bundled Netty artifacts must resolve to `4.2.15.Final`.
- No `netty-4.1.x` artifacts should remain.
- Startup scripts must retain `-Dio.netty.allocator.type=pooled`.
### Ⅴ. Special notes for reviews
- Netty 4.2 is not intended to coexist with Netty 4.1 in the same runtime classpath. This PR centralizes version resolution through the Netty BOM and overrides Spring Boot-managed Netty versions where necessary.
- The migration is intentionally scoped to deprecated event-loop APIs and direct internal-API usage; business protocol behavior is unchanged.
- The SkyWalking plugin now declares direct `provided` Netty dependencies because it directly uses Netty `Channel` and `AttributeMap` types.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#issuecomment-5380462411
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `60.00000%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 73.26%. Comparing base ([`dbea185`](https://app.codecov.io/gh/apache/incubator-seata/commit/dbea1859ef46da881310ccf2dd0b3a27dcca22c8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0ecdd74`](https://app.codecov.io/gh/apache/incubator-seata/commit/0ecdd74883e513fa055c903c9452a0bc5b8c888a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/seata/core/rpc/netty/NettyClientBootstrap.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyClientBootstrap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlDbGllbnRCb290c3RyYXAuamF2YQ==) | 44.44% | [4 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...che/seata/core/rpc/netty/NettyServerBootstrap.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyServerBootstrap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlTZXJ2ZXJCb290c3RyYXAuamF2YQ==) | 58.33% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8209 +/- ##
=========================================
Coverage 73.25% 73.26%
Complexity 1146 1146
=========================================
Files 1153 1153
Lines 42348 42359 +11
Branches 5061 5061
=========================================
+ Hits 31022 31034 +12
- Misses 8845 8846 +1
+ Partials 2481 2479 -2
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...g/apache/seata/config/etcd3/EtcdConfiguration.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=config%2Fseata-config-etcd3%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fconfig%2Fetcd3%2FEtcdConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29uZmlnL3NlYXRhLWNvbmZpZy1ldGNkMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29uZmlnL2V0Y2QzL0V0Y2RDb25maWd1cmF0aW9uLmphdmE=) | `46.44% <100.00%> (ø)` | |
| [...che/seata/core/protocol/detector/HttpDetector.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Fprotocol%2Fdetector%2FHttpDetector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9wcm90b2NvbC9kZXRlY3Rvci9IdHRwRGV0ZWN0b3IuamF2YQ==) | `82.50% <100.00%> (+0.92%)` | :arrow_up: |
| [...g/apache/seata/core/rpc/netty/NettyBaseConfig.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyBaseConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlCYXNlQ29uZmlnLmphdmE=) | `75.75% <100.00%> (+3.03%)` | :arrow_up: |
| [...che/seata/core/rpc/netty/NettyClientBootstrap.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyClientBootstrap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlDbGllbnRCb290c3RyYXAuamF2YQ==) | `54.63% <44.44%> (+0.38%)` | :arrow_up: |
| [...che/seata/core/rpc/netty/NettyServerBootstrap.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyServerBootstrap.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlTZXJ2ZXJCb290c3RyYXAuamF2YQ==) | `54.36% <58.33%> (+0.76%)` | :arrow_up: |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8209/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8209?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r3878657006
##########
build/pom.xml:
##########
@@ -111,7 +112,7 @@
<maven-checkstyle-plugin.version>3.6.0</maven-checkstyle-plugin.version>
<spotless-maven-plugin.version>2.44.3</spotless-maven-plugin.version>
<palantirJavaFormat.version>2.38.0</palantirJavaFormat.version>
- <maven-enforcer-plugin.version>3.0.0-M3</maven-enforcer-plugin.version>
+ <maven-enforcer-plugin.version>3.6.3</maven-enforcer-plugin.version>
Review Comment:
This plugin release requires Maven 3.6.3, but the build still explicitly accepts Maven 3.6.0 and newer (`requireMavenVersion` below). On Maven 3.6.0–3.6.2, Maven can fail to load the plugin before that rule runs. Please either raise the declared minimum to 3.6.3 or retain an Enforcer Plugin version compatible with Maven 3.6.0.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4208323017
##########
extensions/rpc/seata-grpc/src/test/java/org/apache/seata/integration/grpc/interceptor/GrpcTest.java:
##########
@@ -20,64 +20,62 @@
import io.grpc.ClientInterceptors;
import io.grpc.ManagedChannel;
import io.grpc.Metadata;
+import io.grpc.Server;
import io.grpc.ServerInterceptor;
import io.grpc.ServerInterceptors;
-import io.grpc.inprocess.InProcessChannelBuilder;
-import io.grpc.inprocess.InProcessServerBuilder;
+import io.grpc.netty.NettyChannelBuilder;
+import io.grpc.netty.NettyServerBuilder;
import io.grpc.stub.StreamObserver;
-import io.grpc.testing.GrpcCleanupRule;
import org.apache.seata.core.context.RootContext;
import org.apache.seata.core.model.BranchType;
import org.apache.seata.integration.grpc.interceptor.client.ClientTransactionInterceptor;
import org.apache.seata.integration.grpc.interceptor.proto.ContextRpcGrpc;
import org.apache.seata.integration.grpc.interceptor.proto.Request;
import org.apache.seata.integration.grpc.interceptor.proto.Response;
import org.apache.seata.integration.grpc.interceptor.server.ServerTransactionInterceptor;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executor;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.AdditionalAnswers.delegatesTo;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
public class GrpcTest {
- @Rule
- public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule();
-
private final ServerInterceptor mockServerInterceptor =
mock(ServerInterceptor.class, delegatesTo(new ServerTransactionInterceptor()));
private static final String XID = "192.168.0.1:8091:10086";
+ private Server server;
+ private ManagedChannel channel;
+
+ @AfterEach
+ void tearDown() throws InterruptedException {
+ RootContext.unbind();
+ RootContext.unbindBranchType();
+ if (channel != null) {
+ channel.shutdownNow().awaitTermination(5, TimeUnit.SECONDS);
+ }
+ if (server != null) {
+ server.shutdownNow().awaitTermination(5, TimeUnit.SECONDS);
+ }
+ }
Review Comment:
Changed teardown to attempt graceful shutdown before falling back to forced shutdown.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#issuecomment-6091951742
> Please keep the client path working on Netty 4.1: with this change, TM/RM fail to start in Spring Boot 2.x/3.x applications, whose BOM pins Netty 4.1.
>
> **What I ran locally**
>
> I built `seata-core` from this PR ([0e7ee0d](https://github.com/apache/incubator-seata/commit/0e7ee0d8e5c6c7bee5b6ea2dc2b38091f7d1d971)) and from 2.x ([53d4b19](https://github.com/apache/incubator-seata/commit/53d4b19a3811787e066cef7b040abccdcb001aee)). Then I created and started a TM `NettyClientBootstrap` with each Netty version on the classpath. 4.1.101.Final is the version that Spring Boot 2.7.18 and 3.2.0 manage.
>
> seata-core Netty 4.1.101.Final Netty 4.2.15.Final
> 2.x ([53d4b19](https://github.com/apache/incubator-seata/commit/53d4b19a3811787e066cef7b040abccdcb001aee)) starts starts
> this PR ([0e7ee0d](https://github.com/apache/incubator-seata/commit/0e7ee0d8e5c6c7bee5b6ea2dc2b38091f7d1d971)) `NoClassDefFoundError` starts
> ```
> java.lang.NoClassDefFoundError: io/netty/channel/MultiThreadIoEventLoopGroup
> at org.apache.seata.core.rpc.netty.NettyClientBootstrap.createEventLoopGroupWorker(NettyClientBootstrap.java:246)
> at org.apache.seata.core.rpc.netty.NettyClientBootstrap.<init>(NettyClientBootstrap.java:91)
> Caused by: java.lang.ClassNotFoundException: io.netty.channel.MultiThreadIoEventLoopGroup
> ```
>
> 2.x works on both versions because `NioEventLoopGroup` and `EpollEventLoopGroup` still exist in 4.2 (deprecated).
>
> CI doesn't catch this. `seata-dependencies` now imports `netty-bom` before the Spring Boot BOM, so the `-Dspring-boot.version` matrix no longer changes the Netty version. Output of `mvn -pl core -am dependency:list -DincludeGroupIds=io.netty -P args-for-client-test`:
>
> `-Dspring-boot.version=2.7.18` `-Dspring-boot.version=3.3.0`
> 2.x 4.1.101.Final 4.1.110.Final
> this PR 4.2.15.Final 4.2.15.Final
> A user's application is different: its own `spring-boot-dependencies` wins over the 4.2.15 that Seata brings in transitively, so the application gets Netty 4.1.x at runtime.
Netty 4.2 and 4.1 don't appear to be fully compatible, so I think we should consider dropping support for Netty 4.1. Major RPC frameworks, such as Dubbo, have already upgraded to Netty 4.2.
As for potential dependency conflicts, we should clearly document them in the upgrade notes and advise users to upgrade to Netty 4.2.
Of course, if we can maintain backward compatibility between Netty 4.2 and 4.1, that would be the ideal outcome.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4080885912
##########
core/src/test/java/org/apache/seata/core/rpc/netty/NettyClientBootstrapTest.java:
##########
@@ -90,4 +112,14 @@ private EventLoopGroup getEventLoopGroupWorker(NettyClientBootstrap bootstrap) {
throw new RuntimeException(e);
}
}
+
+ private Bootstrap getBootstrap(NettyClientBootstrap bootstrap) {
+ try {
+ java.lang.reflect.Field field = NettyClientBootstrap.class.getDeclaredField("bootstrap");
+ field.setAccessible(true);
+ return (Bootstrap) field.get(bootstrap);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
Review Comment:
Similar to the server bootstrap test, this reflection helper wraps all failures in a generic `RuntimeException`. Consider using a test assertion failure with a descriptive message (e.g., noting the field name `bootstrap`) to make debugging clearer if the field is renamed or access changes.
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientBootstrap.java:
##########
@@ -123,14 +124,15 @@ public void start() {
.option(ChannelOption.SO_KEEPALIVE, true)
.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, nettyClientConfig.getConnectTimeoutMillis())
.option(ChannelOption.SO_SNDBUF, nettyClientConfig.getClientSocketSndBufSize())
- .option(ChannelOption.SO_RCVBUF, nettyClientConfig.getClientSocketRcvBufSize());
+ .option(ChannelOption.SO_RCVBUF, nettyClientConfig.getClientSocketRcvBufSize())
+ .option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
- if (PlatformDependent.isWindows() || PlatformDependent.isOsx()) {
- LOGGER.info("client run on MacOS/Windows, fallback to NIO.");
- } else if (Epoll.isAvailable()) {
+ if (Epoll.isAvailable()) {
bootstrap
.option(EpollChannelOption.EPOLL_MODE, EpollMode.EDGE_TRIGGERED)
.option(EpollChannelOption.TCP_QUICKACK, true);
+ } else if (LOGGER.isInfoEnabled()) {
+ LOGGER.info("Epoll is unavailable, fallback to NIO: {}", Epoll.unavailabilityCause());
}
Review Comment:
Epoll-specific channel options are applied based solely on `Epoll.isAvailable()`, which can diverge from the channel/event-loop actually chosen by configuration (e.g., if epoll is available but disabled/forced to NIO). This can lead to noisy 'unknown channel option' warnings or ineffective configuration. Consider guarding these options behind the same decision used to select the epoll channel/event-loop (e.g., `NettyServerConfig.enableEpoll()` and/or checking the selected channel class) and only logging the fallback when epoll was intended to be used.
##########
extensions/rpc/seata-grpc/src/test/java/org/apache/seata/integration/grpc/interceptor/GrpcTest.java:
##########
@@ -20,64 +20,62 @@
import io.grpc.ClientInterceptors;
import io.grpc.ManagedChannel;
import io.grpc.Metadata;
+import io.grpc.Server;
import io.grpc.ServerInterceptor;
import io.grpc.ServerInterceptors;
-import io.grpc.inprocess.InProcessChannelBuilder;
-import io.grpc.inprocess.InProcessServerBuilder;
+import io.grpc.netty.NettyChannelBuilder;
+import io.grpc.netty.NettyServerBuilder;
import io.grpc.stub.StreamObserver;
-import io.grpc.testing.GrpcCleanupRule;
import org.apache.seata.core.context.RootContext;
import org.apache.seata.core.model.BranchType;
import org.apache.seata.integration.grpc.interceptor.client.ClientTransactionInterceptor;
import org.apache.seata.integration.grpc.interceptor.proto.ContextRpcGrpc;
import org.apache.seata.integration.grpc.interceptor.proto.Request;
import org.apache.seata.integration.grpc.interceptor.proto.Response;
import org.apache.seata.integration.grpc.interceptor.server.ServerTransactionInterceptor;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executor;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
-import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.AdditionalAnswers.delegatesTo;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
public class GrpcTest {
- @Rule
- public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule();
-
private final ServerInterceptor mockServerInterceptor =
mock(ServerInterceptor.class, delegatesTo(new ServerTransactionInterceptor()));
private static final String XID = "192.168.0.1:8091:10086";
+ private Server server;
+ private ManagedChannel channel;
+
+ @AfterEach
+ void tearDown() throws InterruptedException {
+ RootContext.unbind();
+ RootContext.unbindBranchType();
+ if (channel != null) {
+ channel.shutdownNow().awaitTermination(5, TimeUnit.SECONDS);
+ }
+ if (server != null) {
+ server.shutdownNow().awaitTermination(5, TimeUnit.SECONDS);
+ }
+ }
Review Comment:
Using `shutdownNow()` for both channel and server can make test cleanup more abrupt and potentially flaky under load (it cancels in-flight work). Consider using graceful `shutdown()` first (with a bounded `awaitTermination`), and only falling back to `shutdownNow()` if termination times out; this tends to reduce intermittent failures while still keeping cleanup bounded.
##########
core/src/test/java/org/apache/seata/core/rpc/netty/NettyServerBootstrapTest.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.core.rpc.netty;
+
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.MultiThreadIoEventLoopGroup;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+class NettyServerBootstrapTest {
+
+ @Test
+ void testNioEventLoopGroups() {
+ NettyServerBootstrap bootstrap;
+ try (MockedStatic<NettyServerConfig> mockedConfig =
+ Mockito.mockStatic(NettyServerConfig.class, Mockito.CALLS_REAL_METHODS)) {
+ mockedConfig.when(NettyServerConfig::enableEpoll).thenReturn(false);
+ bootstrap = new NettyServerBootstrap(new NettyServerConfig());
+ }
+
+ EventLoopGroup bossGroup = getEventLoopGroup(bootstrap, "eventLoopGroupBoss");
+ EventLoopGroup workerGroup = getEventLoopGroup(bootstrap, "eventLoopGroupWorker");
+ try {
+ Assertions.assertInstanceOf(MultiThreadIoEventLoopGroup.class, bossGroup);
+ Assertions.assertInstanceOf(MultiThreadIoEventLoopGroup.class, workerGroup);
+ } finally {
+ bossGroup.shutdownGracefully().syncUninterruptibly();
+ workerGroup.shutdownGracefully().syncUninterruptibly();
+ }
+ }
+
+ private EventLoopGroup getEventLoopGroup(NettyServerBootstrap bootstrap, String fieldName) {
+ try {
+ java.lang.reflect.Field field = NettyServerBootstrap.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ return (EventLoopGroup) field.get(bootstrap);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
Review Comment:
The reflection helper throws a generic `RuntimeException` without contextual details, which makes failures harder to diagnose (especially when multiple fields are accessed). Consider including `fieldName` in the exception message and/or failing via JUnit assertions (e.g., `Assertions.fail(...)`) so the test output clearly indicates which field access failed.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#issuecomment-6092257006
@funky-eyes Ah, I was hoping we could keep this upgrade seamless for users, but I guess it can't be helped... Understood.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4208314397
##########
core/src/test/java/org/apache/seata/core/rpc/netty/NettyClientBootstrapTest.java:
##########
@@ -90,4 +112,14 @@ private EventLoopGroup getEventLoopGroupWorker(NettyClientBootstrap bootstrap) {
throw new RuntimeException(e);
}
}
+
+ private Bootstrap getBootstrap(NettyClientBootstrap bootstrap) {
+ try {
+ java.lang.reflect.Field field = NettyClientBootstrap.class.getDeclaredField("bootstrap");
+ field.setAccessible(true);
+ return (Bootstrap) field.get(bootstrap);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
Review Comment:
Added descriptive error messages while preserving the original cause.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4208318553
##########
core/src/test/java/org/apache/seata/core/rpc/netty/NettyServerBootstrapTest.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.core.rpc.netty;
+
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.MultiThreadIoEventLoopGroup;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+class NettyServerBootstrapTest {
+
+ @Test
+ void testNioEventLoopGroups() {
+ NettyServerBootstrap bootstrap;
+ try (MockedStatic<NettyServerConfig> mockedConfig =
+ Mockito.mockStatic(NettyServerConfig.class, Mockito.CALLS_REAL_METHODS)) {
+ mockedConfig.when(NettyServerConfig::enableEpoll).thenReturn(false);
+ bootstrap = new NettyServerBootstrap(new NettyServerConfig());
+ }
+
+ EventLoopGroup bossGroup = getEventLoopGroup(bootstrap, "eventLoopGroupBoss");
+ EventLoopGroup workerGroup = getEventLoopGroup(bootstrap, "eventLoopGroupWorker");
+ try {
+ Assertions.assertInstanceOf(MultiThreadIoEventLoopGroup.class, bossGroup);
+ Assertions.assertInstanceOf(MultiThreadIoEventLoopGroup.class, workerGroup);
+ } finally {
+ bossGroup.shutdownGracefully().syncUninterruptibly();
+ workerGroup.shutdownGracefully().syncUninterruptibly();
+ }
+ }
+
+ private EventLoopGroup getEventLoopGroup(NettyServerBootstrap bootstrap, String fieldName) {
+ try {
+ java.lang.reflect.Field field = NettyServerBootstrap.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ return (EventLoopGroup) field.get(bootstrap);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
Review Comment:
Added the field name to the error message while preserving the original cause.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4208306531
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientBootstrap.java:
##########
@@ -123,14 +124,15 @@ public void start() {
.option(ChannelOption.SO_KEEPALIVE, true)
.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, nettyClientConfig.getConnectTimeoutMillis())
.option(ChannelOption.SO_SNDBUF, nettyClientConfig.getClientSocketSndBufSize())
- .option(ChannelOption.SO_RCVBUF, nettyClientConfig.getClientSocketRcvBufSize());
+ .option(ChannelOption.SO_RCVBUF, nettyClientConfig.getClientSocketRcvBufSize())
+ .option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
- if (PlatformDependent.isWindows() || PlatformDependent.isOsx()) {
- LOGGER.info("client run on MacOS/Windows, fallback to NIO.");
- } else if (Epoll.isAvailable()) {
+ if (Epoll.isAvailable()) {
bootstrap
.option(EpollChannelOption.EPOLL_MODE, EpollMode.EDGE_TRIGGERED)
.option(EpollChannelOption.TCP_QUICKACK, true);
+ } else if (LOGGER.isInfoEnabled()) {
+ LOGGER.info("Epoll is unavailable, fallback to NIO: {}", Epoll.unavailabilityCause());
}
Review Comment:
Epoll-specific options now depend on the selected channel type. Added a regression test for NIO channels.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#issuecomment-6040357916
> Could you confirm whether seata-console also needs to be aligned with Netty 4.2.15? Under the source-release profile, its effective dependency tree still resolves io.netty:* to 4.2.12.Final via Spring Boot 4.0.6. If only the final server/namingserver distributions need alignment and they already resolve to 4.2.15, no change is needed; otherwise, console should import the Netty BOM before Spring Boot as well.
Added the Netty BOM before the Spring Boot BOM to align Console with Netty 4.2.15.Final.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "xjlgod (via GitHub)" <gi...@apache.org>.
xjlgod commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4208299023
##########
build/pom.xml:
##########
@@ -111,7 +112,7 @@
<maven-checkstyle-plugin.version>3.6.0</maven-checkstyle-plugin.version>
<spotless-maven-plugin.version>2.44.3</spotless-maven-plugin.version>
<palantirJavaFormat.version>2.38.0</palantirJavaFormat.version>
- <maven-enforcer-plugin.version>3.0.0-M3</maven-enforcer-plugin.version>
+ <maven-enforcer-plugin.version>3.6.3</maven-enforcer-plugin.version>
Review Comment:
Reverted the Enforcer upgrade and the added convergence rules to keep this PR focused. The original Maven requirement remains unchanged.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feature: upgrade Netty to 4.2.15.Final [incubator-seata]
Posted by "slievrly (via GitHub)" <gi...@apache.org>.
slievrly commented on code in PR #8209:
URL: https://github.com/apache/incubator-seata/pull/8209#discussion_r4167423738
##########
build/pom.xml:
##########
@@ -83,6 +83,7 @@
<!-- The version of spring-boot for 'spring-boot-dependencies' and 'spring-boot-maven-plugin' -->
<spring-boot.version>2.7.18</spring-boot.version>
<spring-framework-bom.version>5.3.39</spring-framework-bom.version>
+ <netty.version>4.2.15.Final</netty.version>
Review Comment:
[P2] Update the server distribution dependency inventory with this upgrade. distribution/LICENSE-server still lists netty-all and the Netty modules at 4.1.101.Final, and distribution/NOTICE.md still describes the old dependency tree. With the BOM pinning 4.2.15.Final, those shipped inventory entries no longer describe the resolved server libraries. Regenerate/check the actual server dependency tree and update the affected entries, including modules added or removed by Netty 4.2.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org