You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "devjue (via GitHub)" <gi...@apache.org> on 2026/06/16 06:28:19 UTC
[PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
devjue opened a new pull request, #16345:
URL: https://github.com/apache/dubbo/pull/16345
## What is the purpose of the change
Fixes #16344.
Eliminate the single-request failure window that occurs every time a Dubbo Triple Consumer receives an HTTP/2 `GOAWAY(errorCode=0, lastStreamId=MAX_INT)` frame from gateways / reverse proxies / providers that gracefully rotate connections (e.g. `max_requests_per_connection`, `idle_timeout`, hot-restart drain).
Previously, `AbstractNettyConnectionClient#onGoaway` nulled the channel reference immediately and `NettyConnectionHandler#onGoAway` then scheduled a reconnect, leaving a brief window in which `AbstractClusterInvoker#checkInvokers` throws `RpcException` before FailoverCluster's retry loop — so `retries=N` did NOT mitigate the issue. This PR implements graceful migration: keep the old channel serving until a new channel is ready, then atomically swap.
## Brief changelog
- **`NettyConnectionHandler#onGoAway`**: keep old channel alive; schedule `attemptGracefulMigration` on `connectivityExecutor` after `GRACEFUL_RECONNECT_DELAY_MS = 200ms`; one retry on failure, then fall back to `AbstractNettyConnectionClient#onGoaway`.
- **`NettyConnectionClient#initBootstrap`**: `closeFuture` listener now calls `compareAndClearNettyChannel(ch)` instead of `clearNettyChannel()` — prevents the old channel's close listener from wiping a freshly swapped-in new channel.
- **`AbstractNettyConnectionClient`**: add `compareAndClearNettyChannel`, plus package-private `getConnectivityExecutor()` / `getReconnectDuration()` accessors.
- **`NettyChannel#getChannelIfPresent`**: lookup-only API used by `channelInactive` to avoid allocating a transient `NettyChannel` purely for logging.
- **`TripleGoAwayHandler`**: log `errorCode` and `lastStreamId` so the GOAWAY trigger is observable.
## Verifying this change
Validated against a local Triple demo with sustained 1 QPS traffic:
- 26 GOAWAY frames received → 100% graceful migration success → **zero** `No provider available` errors and **zero** request failures.
- Channel rotation chain (first 18 migrations):
`:45956 → :45110 → :53222 → :58366 → :56372 → :55436 → :55188 → :52926 → :50682 → :48682 → :47626 → :44014 → :43116 → :45044 → :48602 → :51880 → :50992 → :51672`
- No regression in existing `dubbo-remoting-netty4` and `dubbo-rpc-triple` unit tests.
## Does this pull request potentially affect one of the following parts
- [ ] Dependencies (does it add or upgrade a dependency)
- [ ] The public API
- [x] The runtime per-connection behavior (HTTP/2 GOAWAY handling on the Consumer side)
- [ ] The persistence format of the configurations
- [ ] The default values of configurations
- [ ] The threading model (uses existing `connectivityExecutor`; does NOT introduce new threads)
- [ ] The serialization protocol
- [ ] The compatibility with previous versions
Backward-compatible: defaults are unchanged; the only observable difference is that GOAWAY frames no longer cause a request failure window.
## Documentation
- Does this pull request introduce a new feature? **No** (bug fix)
- If yes, how is the feature documented? **Not applicable**
Happy to backport to `3.2` once this lands on `3.3`.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "LI123456mo (via GitHub)" <gi...@apache.org>.
LI123456mo commented on code in PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#discussion_r3452305572
##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyConnectionHandler.java:
##########
@@ -76,7 +165,7 @@ public void reconnect(Object channel) {
LOGGER.debug("Connection:{} is reconnecting, attempt={}", connectionClient, 1);
}
if (connectionClient.isClosed()) {
Review Comment:
@devjue , am not seeing this line in the original code
Line 78
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "uuuyuqi (via GitHub)" <gi...@apache.org>.
uuuyuqi commented on PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#issuecomment-4715750306
@chickenlj PTAL
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "LI123456mo (via GitHub)" <gi...@apache.org>.
LI123456mo commented on code in PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#discussion_r3452305572
##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyConnectionHandler.java:
##########
@@ -76,7 +165,7 @@ public void reconnect(Object channel) {
LOGGER.debug("Connection:{} is reconnecting, attempt={}", connectionClient, 1);
}
if (connectionClient.isClosed()) {
Review Comment:
@devjue , am not seeing this line in the original code
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#issuecomment-4931312794
said [nicole-lihui](https://github.com/nicole-lihui):
```
put together a bytecode-injection Java Agent prototype (old channel gets a configurable drain timeout instead of closing immediately; the stream-level GOAWAY user event skips ctx.close()), which can hot-patch an existing 3.2.10 production process without recompiling or restarting it. In my testing this eliminates the failure window entirely.
```
https://github.com/nicole-lihui/dubbo32-fix-goaway-agent
@devjue PTAL
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#discussion_r3472029336
##########
dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/TripleGoAwayTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.dubbo.remoting.transport.netty4;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.config.ApplicationConfig;
+import org.apache.dubbo.config.context.ConfigManager;
+import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
+import org.apache.dubbo.remoting.api.connection.ConnectionManager;
+import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
+import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ModuleModel;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.dubbo.common.constants.CommonConstants.EXECUTOR_MANAGEMENT_MODE_DEFAULT;
+import static org.awaitility.Awaitility.await;
+
+public class TripleGoAwayTest {
+
+ private static URL url;
+
+ private static NettyPortUnificationServer server;
+
+ private static ConnectionManager connectionManager;
Review Comment:
To make the test use an isolated model end-to-end (and later tear it down), keep a static FrameworkModel reference rather than relying on the global default model.
##########
dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/TripleGoAwayTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.dubbo.remoting.transport.netty4;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.config.ApplicationConfig;
+import org.apache.dubbo.config.context.ConfigManager;
+import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
+import org.apache.dubbo.remoting.api.connection.ConnectionManager;
+import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
+import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ModuleModel;
+
Review Comment:
This test initializes its scope with ApplicationModel.defaultModel(), which is explicitly discouraged because it shares global process state and can become “broken” during teardown, causing unpredictable cross-test interference. Prefer creating an isolated FrameworkModel + frameworkModel.newApplication(), and destroy the FrameworkModel in `@AfterAll` to clean up shared resources.
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/transport/TripleGoAwayHandler.java:
##########
@@ -35,12 +42,20 @@ public TripleGoAwayHandler() {}
@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (msg instanceof Http2GoAwayFrame) {
+ Http2GoAwayFrame goAwayFrame = (Http2GoAwayFrame) msg;
final ConnectionHandler connectionHandler =
Review Comment:
This handler logs GOAWAY errorCode/lastStreamId but always triggers the same “graceful migration” path for every GOAWAY. For non-advisory GOAWAYs (e.g., errorCode != 0 or lastStreamId != MAX_INT), continuing to route new streams on the old channel until migration completes can increase request failures; consider gating the graceful-migration behavior on these fields and falling back to the original close/clear + reconnect behavior otherwise.
##########
dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/TripleGoAwayTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.dubbo.remoting.transport.netty4;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.config.ApplicationConfig;
+import org.apache.dubbo.config.context.ConfigManager;
+import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
+import org.apache.dubbo.remoting.api.connection.ConnectionManager;
+import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
+import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ModuleModel;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.dubbo.common.constants.CommonConstants.EXECUTOR_MANAGEMENT_MODE_DEFAULT;
+import static org.awaitility.Awaitility.await;
+
+public class TripleGoAwayTest {
+
+ private static URL url;
+
+ private static NettyPortUnificationServer server;
+
+ private static ConnectionManager connectionManager;
+
+ @BeforeAll
+ public static void init() throws Throwable {
+ int port = NetUtils.getAvailablePort();
+ url = URL.valueOf("tri://127.0.0.1:" + port + "?foo=bar");
+ ApplicationModel applicationModel = ApplicationModel.defaultModel();
+ ApplicationConfig applicationConfig = new ApplicationConfig("provider-app");
+ applicationConfig.setExecutorManagementMode(EXECUTOR_MANAGEMENT_MODE_DEFAULT);
+ applicationModel.getApplicationConfigManager().setApplication(applicationConfig);
+ ConfigManager configManager = new ConfigManager(applicationModel);
+ configManager.setApplication(applicationConfig);
+ configManager.getApplication();
+ applicationModel.setConfigManager(configManager);
+ url = url.setScopeModel(applicationModel);
+ ModuleModel moduleModel = applicationModel.getDefaultModule();
+ url = url.putAttribute(CommonConstants.SCOPE_MODEL, moduleModel);
+ server = new NettyPortUnificationServer(url, new DefaultPuHandler());
+ server.bind();
+ connectionManager = url.getOrDefaultFrameworkModel()
+ .getExtensionLoader(ConnectionManager.class)
+ .getExtension(MultiplexProtocolConnectionManager.NAME);
+ }
+
+ @AfterAll
+ public static void close() {
+ try {
+ server.close();
+ } catch (Throwable e) {
+ // ignored
+ }
+ }
Review Comment:
The `@AfterAll` cleanup closes the server but never destroys the FrameworkModel created for the test. Destroying it prevents shared executor/model resources from leaking across the test suite.
##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/AbstractNettyConnectionClient.java:
##########
@@ -229,10 +230,16 @@ public void onConnected(Object channel) {
connectedPromise.trySuccess(null);
if (logger.isDebugEnabled()) {
- logger.debug("Connection:{} connected", this);
+ logger.debug("Connection:{} connected, graceful migration complete", this);
}
Review Comment:
onConnected() is invoked for the initial connect as well as for migrations; the new debug log message always says “graceful migration complete”, which is misleading when this is just the first connection establishment. Consider making the message conditional (only mention migration when there was a previous channel).
##########
dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/TripleGoAwayTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.dubbo.remoting.transport.netty4;
+
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.constants.CommonConstants;
+import org.apache.dubbo.common.utils.NetUtils;
+import org.apache.dubbo.config.ApplicationConfig;
+import org.apache.dubbo.config.context.ConfigManager;
+import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
+import org.apache.dubbo.remoting.api.connection.ConnectionManager;
+import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
+import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+import org.apache.dubbo.rpc.model.ModuleModel;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.dubbo.common.constants.CommonConstants.EXECUTOR_MANAGEMENT_MODE_DEFAULT;
+import static org.awaitility.Awaitility.await;
+
+public class TripleGoAwayTest {
+
+ private static URL url;
+
+ private static NettyPortUnificationServer server;
+
+ private static ConnectionManager connectionManager;
+
+ @BeforeAll
+ public static void init() throws Throwable {
+ int port = NetUtils.getAvailablePort();
+ url = URL.valueOf("tri://127.0.0.1:" + port + "?foo=bar");
+ ApplicationModel applicationModel = ApplicationModel.defaultModel();
+ ApplicationConfig applicationConfig = new ApplicationConfig("provider-app");
Review Comment:
Avoid using ApplicationModel.defaultModel() in tests; create a fresh FrameworkModel and ApplicationModel for isolation. This also makes it possible to destroy the FrameworkModel in `@AfterAll` and prevent global state leakage into other 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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#issuecomment-4777987946
We would appreciated if you add testcases to https://github.com/apache/dubbo-samples for this PR.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#issuecomment-4785697336
> @zrlw , have been going through the PR , seen the changes made , tried to run the changes locally,test the changes, and it worked,
> @Test
> void testGoawayCausesNoChannelFailure() throws Throwable {
> final AbstractConnectionClient connectionClient =
> connectionManager.connect(url, new DefaultPuHandler());
> Assertions.assertTrue(
> connectionClient.isAvailable(),
> "Precondition: connection should be available before GOAWAY"
> );
> io.netty.channel.Channel channelBeforeGoaway = connectionClient.getChannel(true);
> NettyConnectionHandler handler = new NettyConnectionHandler(
> (AbstractNettyConnectionClient) connectionClient);
> handler.onGoAway(channelBeforeGoaway);
> io.netty.channel.Channel channelAfterGoaway = connectionClient.getChannel(true);
> Assertions.assertNotNull(
> channelAfterGoaway,
> "Channel should NOT be null immediately after GOAWAY " +
> "— old channel should stay alive until new one is ready"
> );
> await().atMost(Duration.ofSeconds(10))
> .until(() -> connectionClient.isAvailable());
> Assertions.assertTrue(
> connectionClient.isAvailable(),
> "Connection should be available after GOAWAY migration completes"
> );
> connectionClient.close();
> }
> ```
>
> }
Created TripleGoAwayTest.java based on these codes.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "LI123456mo (via GitHub)" <gi...@apache.org>.
LI123456mo commented on code in PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#discussion_r3452305572
##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyConnectionHandler.java:
##########
@@ -76,7 +165,7 @@ public void reconnect(Object channel) {
LOGGER.debug("Connection:{} is reconnecting, attempt={}", connectionClient, 1);
}
if (connectionClient.isClosed()) {
Review Comment:
@devjue , am not seeing this line in the original code
Line 78
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fix #16344: graceful HTTP/2 GOAWAY migration to eliminate request failure window [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16345:
URL: https://github.com/apache/dubbo/pull/16345#issuecomment-4715853090
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/16345?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 `42.85714%` with `24 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 36.94%. Comparing base ([`866860d`](https://app.codecov.io/gh/apache/dubbo/commit/866860d01395870ce010606f14326c963e040f56?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`02e4fa9`](https://app.codecov.io/gh/apache/dubbo/commit/02e4fa9f1a76a245d914302d9e3cd72d46fe5492?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/dubbo/pull/16345?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...oting/transport/netty4/NettyConnectionHandler.java](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FNettyConnectionHandler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q29ubmVjdGlvbkhhbmRsZXIuamF2YQ==) | 25.92% | [18 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ransport/netty4/AbstractNettyConnectionClient.java](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FAbstractNettyConnectionClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L0Fic3RyYWN0TmV0dHlDb25uZWN0aW9uQ2xpZW50LmphdmE=) | 60.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../dubbo/remoting/transport/netty4/NettyChannel.java](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-netty4%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Ftransport%2Fnetty4%2FNettyChannel.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2hhbm5lbC5qYXZh) | 33.33% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/16345?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (866860d) and HEAD (02e4fa9). Click for more details.
>
> <details><summary>HEAD has 5 uploads less than BASE</summary>
>
>| Flag | BASE (866860d) | HEAD (02e4fa9) |
>|------|------|------|
>|unit-tests-java17|1|0|
>|unit-tests-java11|1|0|
>|unit-tests-java8|1|0|
>|unit-tests-java25|1|0|
>|unit-tests-java21|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.3 #16345 +/- ##
=============================================
- Coverage 60.80% 36.94% -23.87%
- Complexity 15 11756 +11741
=============================================
Files 1953 1952 -1
Lines 89208 89217 +9
Branches 13458 13375 -83
=============================================
- Hits 54245 32963 -21282
- Misses 29397 51670 +22273
+ Partials 5566 4584 -982
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.14% <40.47%> (+0.03%)` | :arrow_up: |
| [integration-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.30% <40.47%> (+<0.01%)` | :arrow_up: |
| [samples-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.17% <14.28%> (-0.01%)` | :arrow_down: |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.82% <11.90%> (+0.10%)` | :arrow_up: |
| [unit-tests-java11](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java17](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java25](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo/pull/16345?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org