From e5d2a5798e5e6bf99bb5f42fc82b1741965ffa35 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Tue, 23 May 2023 14:52:49 +0800 Subject: [PATCH] Support config close timeout (#12386) * Support config close timeout * Fix spell * fix ut --- .../org/apache/dubbo/remoting/Constants.java | 2 ++ .../support/header/CloseTimerTask.java | 14 +++++------ .../support/header/HeaderExchangeServer.java | 17 ++++++------- .../apache/dubbo/remoting/utils/UrlUtils.java | 21 ++++++++++++++++ .../dubbo/remoting/utils/UrlUtilsTest.java | 24 +++++++++++++++++++ 5 files changed, 61 insertions(+), 17 deletions(-) diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java index 9920ab126a..bd2ad05220 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java @@ -144,8 +144,10 @@ public interface Constants { String TELNET_KEY = "telnet"; String HEARTBEAT_KEY = "heartbeat"; String HEARTBEAT_CONFIG_KEY = "dubbo.protocol.default-heartbeat"; + String CLOSE_TIMEOUT_CONFIG_KEY = "dubbo.protocol.default-close-timeout"; int DEFAULT_HEARTBEAT = 60 * 1000; String HEARTBEAT_TIMEOUT_KEY = "heartbeat.timeout"; + String CLOSE_TIMEOUT_KEY = "close.timeout"; String CONNECTIONS_KEY = "connections"; int DEFAULT_BACKLOG = 1024; diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java index 7c2eea2289..aa4df8d053 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java @@ -32,11 +32,11 @@ public class CloseTimerTask extends AbstractTimerTask { private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(CloseTimerTask.class); - private final int idleTimeout; + private final int closeTimeout; - public CloseTimerTask(ChannelProvider channelProvider, HashedWheelTimer hashedWheelTimer, Long heartbeatTimeoutTick, int idleTimeout) { - super(channelProvider, hashedWheelTimer, heartbeatTimeoutTick); - this.idleTimeout = idleTimeout; + public CloseTimerTask(ChannelProvider channelProvider, HashedWheelTimer hashedWheelTimer, Long tick, int closeTimeout) { + super(channelProvider, hashedWheelTimer, tick); + this.closeTimeout = closeTimeout; } @Override @@ -46,9 +46,9 @@ public class CloseTimerTask extends AbstractTimerTask { Long lastWrite = lastWrite(channel); Long now = now(); // check ping & pong at server - if ((lastRead != null && now - lastRead > idleTimeout) - || (lastWrite != null && now - lastWrite > idleTimeout)) { - logger.warn(PROTOCOL_FAILED_RESPONSE, "", "", "Close channel " + channel + ", because idleCheck timeout: " + idleTimeout + "ms"); + if ((lastRead != null && now - lastRead > closeTimeout) + || (lastWrite != null && now - lastWrite > closeTimeout)) { + logger.warn(PROTOCOL_FAILED_RESPONSE, "", "", "Close channel " + channel + ", because idleCheck timeout: " + closeTimeout + "ms"); channel.close(); } } catch (Throwable t) { diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeServer.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeServer.java index f27aae80e8..3dca5a9746 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeServer.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeaderExchangeServer.java @@ -49,8 +49,7 @@ import static org.apache.dubbo.common.constants.LoggerCodeConstants.TRANSPORT_FA import static org.apache.dubbo.remoting.Constants.HEARTBEAT_CHECK_TICK; import static org.apache.dubbo.remoting.Constants.LEAST_HEARTBEAT_DURATION; import static org.apache.dubbo.remoting.Constants.TICKS_PER_WHEEL; -import static org.apache.dubbo.remoting.utils.UrlUtils.getHeartbeat; -import static org.apache.dubbo.remoting.utils.UrlUtils.getIdleTimeout; +import static org.apache.dubbo.remoting.utils.UrlUtils.getCloseTimeout; /** * ExchangeServerImpl @@ -211,11 +210,9 @@ public class HeaderExchangeServer implements ExchangeServer { public void reset(URL url) { server.reset(url); try { - int currHeartbeat = getHeartbeat(getUrl()); - int currIdleTimeout = getIdleTimeout(getUrl()); - int heartbeat = getHeartbeat(url); - int idleTimeout = getIdleTimeout(url); - if (currHeartbeat != heartbeat || currIdleTimeout != idleTimeout) { + int currCloseTimeout = getCloseTimeout(getUrl()); + int closeTimeout = getCloseTimeout(url); + if (closeTimeout != currCloseTimeout) { cancelCloseTask(); startIdleCheckTask(url); } @@ -262,9 +259,9 @@ public class HeaderExchangeServer implements ExchangeServer { private void startIdleCheckTask(URL url) { if (!server.canHandleIdle()) { AbstractTimerTask.ChannelProvider cp = () -> unmodifiableCollection(HeaderExchangeServer.this.getChannels()); - int idleTimeout = getIdleTimeout(url); - long idleTimeoutTick = calculateLeastDuration(idleTimeout); - this.closeTimer = new CloseTimerTask(cp, IDLE_CHECK_TIMER.get(), idleTimeoutTick, idleTimeout); + int closeTimeout = getCloseTimeout(url); + long closeTimeoutTick = calculateLeastDuration(closeTimeout); + this.closeTimer = new CloseTimerTask(cp, IDLE_CHECK_TIMER.get(), closeTimeoutTick, closeTimeout); } } } diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/utils/UrlUtils.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/utils/UrlUtils.java index fe620b613f..e3f510c7cc 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/utils/UrlUtils.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/utils/UrlUtils.java @@ -36,6 +36,27 @@ import static org.apache.dubbo.remoting.Constants.SERIALIZATION_KEY; public class UrlUtils { private static final String ALLOWED_SERIALIZATION_KEY = "allowedSerialization"; + public static int getCloseTimeout(URL url) { + String configuredCloseTimeout = System.getProperty(Constants.CLOSE_TIMEOUT_CONFIG_KEY); + int defaultCloseTimeout = -1; + if (StringUtils.isNotEmpty(configuredCloseTimeout)) { + try { + defaultCloseTimeout = Integer.parseInt(configuredCloseTimeout); + } catch (NumberFormatException e) { + // use default heartbeat + } + } + if (defaultCloseTimeout < 0) { + defaultCloseTimeout = getIdleTimeout(url); + } + int closeTimeout = url.getParameter(Constants.CLOSE_TIMEOUT_KEY, defaultCloseTimeout); + int heartbeat = getHeartbeat(url); + if (closeTimeout < heartbeat * 2) { + throw new IllegalStateException("closeTimeout < heartbeatInterval * 2"); + } + return closeTimeout; + } + public static int getIdleTimeout(URL url) { int heartBeat = getHeartbeat(url); // idleTimeout should be at least more than twice heartBeat because possible retries of client. diff --git a/dubbo-remoting/dubbo-remoting-api/src/test/java/org/apache/dubbo/remoting/utils/UrlUtilsTest.java b/dubbo-remoting/dubbo-remoting-api/src/test/java/org/apache/dubbo/remoting/utils/UrlUtilsTest.java index 2ba6f2b506..6a957aee5e 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/test/java/org/apache/dubbo/remoting/utils/UrlUtilsTest.java +++ b/dubbo-remoting/dubbo-remoting-api/src/test/java/org/apache/dubbo/remoting/utils/UrlUtilsTest.java @@ -46,4 +46,28 @@ class UrlUtilsTest { Assertions.assertEquals(200, UrlUtils.getHeartbeat(url)); System.clearProperty(Constants.HEARTBEAT_CONFIG_KEY); } + + @Test + void testGetCloseTimeout() { + URL url1 = URL.valueOf("dubbo://127.0.0.1:12345?heartbeat=10000"); + URL url2 = URL.valueOf("dubbo://127.0.0.1:12345?heartbeat=10000&heartbeat.timeout=50000"); + URL url3 = URL.valueOf("dubbo://127.0.0.1:12345?heartbeat=10000&heartbeat.timeout=10000"); + URL url4 = URL.valueOf("dubbo://127.0.0.1:12345?close.timeout=30000&heartbeat=10000&heartbeat.timeout=10000"); + URL url5 = URL.valueOf("dubbo://127.0.0.1:12345?close.timeout=40000&heartbeat=10000&heartbeat.timeout=50000"); + URL url6 = URL.valueOf("dubbo://127.0.0.1:12345?close.timeout=10000&heartbeat=10000&heartbeat.timeout=10000"); + Assertions.assertEquals(30000, UrlUtils.getCloseTimeout(url1)); + Assertions.assertEquals(50000, UrlUtils.getCloseTimeout(url2)); + Assertions.assertThrows(RuntimeException.class, () -> UrlUtils.getCloseTimeout(url3)); + Assertions.assertThrows(RuntimeException.class, () -> UrlUtils.getCloseTimeout(url4)); + Assertions.assertEquals(40000, UrlUtils.getCloseTimeout(url5)); + Assertions.assertThrows(RuntimeException.class, () -> UrlUtils.getCloseTimeout(url6)); + } + + @Test + void testConfiguredClose() { + System.setProperty(Constants.CLOSE_TIMEOUT_CONFIG_KEY, "180000"); + URL url = URL.valueOf("dubbo://127.0.0.1:12345"); + Assertions.assertEquals(180000, UrlUtils.getCloseTimeout(url)); + System.clearProperty(Constants.HEARTBEAT_CONFIG_KEY); + } }