From fad88db88e3d23c46cf447db25b7a993358a0425 Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 8 Jan 2019 15:34:15 +0800 Subject: [PATCH 1/5] qos heart question fix #3165 --- .../dubbo/qos/server/handler/QosProcessHandler.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java index 80274173c66..796e511c6a0 100644 --- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java +++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java @@ -93,6 +93,15 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) t } } + @Override + public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { + ScheduledFuture future = welcomeFuture; + if(future != null && !future.isCancelled()){ + future.cancel(true); + } + ctx.close(); + } + // G for GET, and P for POST private static boolean isHttp(int magic) { return magic == 'G' || magic == 'P'; From 16dd280f3492afd574d17777153a5a1acf0a58be Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 8 Jan 2019 15:35:37 +0800 Subject: [PATCH 2/5] modify --- .../org/apache/dubbo/qos/server/handler/QosProcessHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java index 796e511c6a0..c0e7a4c3202 100644 --- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java +++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java @@ -96,7 +96,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) t @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { ScheduledFuture future = welcomeFuture; - if(future != null && !future.isCancelled()){ + if (future != null && !future.isCancelled()) { future.cancel(true); } ctx.close(); From 01a4111ca7ee332642a69400f09dd5055480808a Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 8 Jan 2019 16:14:46 +0800 Subject: [PATCH 3/5] judge if it's a IdleStateEvent --- .../dubbo/qos/server/handler/QosProcessHandler.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java index c0e7a4c3202..9a529727350 100644 --- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java +++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java @@ -26,6 +26,7 @@ import io.netty.handler.codec.http.HttpServerCodec; import io.netty.handler.codec.string.StringDecoder; import io.netty.handler.codec.string.StringEncoder; +import io.netty.handler.timeout.IdleStateEvent; import io.netty.handler.timeout.IdleStateHandler; import io.netty.util.CharsetUtil; import io.netty.util.concurrent.ScheduledFuture; @@ -95,11 +96,13 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) t @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { - ScheduledFuture future = welcomeFuture; - if (future != null && !future.isCancelled()) { - future.cancel(true); + if (evt instanceof IdleStateEvent) { + ScheduledFuture future = welcomeFuture; + if (future != null && !future.isCancelled()) { + future.cancel(true); + } + ctx.close(); } - ctx.close(); } // G for GET, and P for POST From fbdb688e356e8222115d35a65851076786691875 Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 8 Jan 2019 17:01:11 +0800 Subject: [PATCH 4/5] add UT --- .../dubbo/common/utils/ExecutorUtil.java | 18 +++++++++++------- .../dubbo/monitor/dubbo/DubboMonitor.java | 3 ++- .../qos/server/handler/QosProcessHandler.java | 6 ++---- .../dubbo/registry/dubbo/DubboRegistry.java | 5 +---- .../registry/multicast/MulticastRegistry.java | 8 +++----- .../exchange/support/FileExchangeGroup.java | 5 ++--- .../dubbo/remoting/p2p/support/FileGroup.java | 5 ++--- 7 files changed, 23 insertions(+), 27 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java index d33af364247..c9f3e39e989 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java @@ -21,11 +21,7 @@ import org.apache.dubbo.common.logger.Logger; import org.apache.dubbo.common.logger.LoggerFactory; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; public class ExecutorUtil { private static final Logger logger = LoggerFactory.getLogger(ExecutorUtil.class); @@ -45,9 +41,10 @@ public static boolean isTerminated(Executor executor) { /** * Use the shutdown pattern from: - * https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ExecutorService.html + * https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ExecutorService.html + * * @param executor the Executor to shutdown - * @param timeout the timeout in milliseconds before termination + * @param timeout the timeout in milliseconds before termination */ public static void gracefulShutdown(Executor executor, int timeout) { if (!(executor instanceof ExecutorService) || isTerminated(executor)) { @@ -131,4 +128,11 @@ public static URL setThreadName(URL url, String defaultName) { url = url.addParameter(Constants.THREAD_NAME_KEY, name); return url; } + + public static void cancelScheduledFuture(ScheduledFuture scheduledFuture) { + ScheduledFuture future = scheduledFuture; + if (future != null && !future.isCancelled()) { + future.cancel(true); + } + } } diff --git a/dubbo-monitor/dubbo-monitor-default/src/main/java/org/apache/dubbo/monitor/dubbo/DubboMonitor.java b/dubbo-monitor/dubbo-monitor-default/src/main/java/org/apache/dubbo/monitor/dubbo/DubboMonitor.java index 04df72df97f..f7f0d87d8e1 100644 --- a/dubbo-monitor/dubbo-monitor-default/src/main/java/org/apache/dubbo/monitor/dubbo/DubboMonitor.java +++ b/dubbo-monitor/dubbo-monitor-default/src/main/java/org/apache/dubbo/monitor/dubbo/DubboMonitor.java @@ -20,6 +20,7 @@ import org.apache.dubbo.common.URL; import org.apache.dubbo.common.logger.Logger; import org.apache.dubbo.common.logger.LoggerFactory; +import org.apache.dubbo.common.utils.ExecutorUtil; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.monitor.Monitor; import org.apache.dubbo.monitor.MonitorService; @@ -209,7 +210,7 @@ public boolean isAvailable() { @Override public void destroy() { try { - sendFuture.cancel(true); + ExecutorUtil.cancelScheduledFuture(sendFuture); } catch (Throwable t) { logger.error("Unexpected error occur at cancel sender timer, cause: " + t.getMessage(), t); } diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java index 9a529727350..9312bb7e4f6 100644 --- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java +++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/handler/QosProcessHandler.java @@ -30,6 +30,7 @@ import io.netty.handler.timeout.IdleStateHandler; import io.netty.util.CharsetUtil; import io.netty.util.concurrent.ScheduledFuture; +import org.apache.dubbo.common.utils.ExecutorUtil; import java.util.List; import java.util.concurrent.TimeUnit; @@ -97,10 +98,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List out) t @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { if (evt instanceof IdleStateEvent) { - ScheduledFuture future = welcomeFuture; - if (future != null && !future.isCancelled()) { - future.cancel(true); - } + ExecutorUtil.cancelScheduledFuture(welcomeFuture); ctx.close(); } } diff --git a/dubbo-registry/dubbo-registry-default/src/main/java/org/apache/dubbo/registry/dubbo/DubboRegistry.java b/dubbo-registry/dubbo-registry-default/src/main/java/org/apache/dubbo/registry/dubbo/DubboRegistry.java index 72604d64474..c936d2979be 100644 --- a/dubbo-registry/dubbo-registry-default/src/main/java/org/apache/dubbo/registry/dubbo/DubboRegistry.java +++ b/dubbo-registry/dubbo-registry-default/src/main/java/org/apache/dubbo/registry/dubbo/DubboRegistry.java @@ -38,7 +38,6 @@ /** * DubboRegistry - * */ public class DubboRegistry extends FailbackRegistry { @@ -124,9 +123,7 @@ public void destroy() { super.destroy(); try { // Cancel the reconnection timer - if (!reconnectFuture.isCancelled()) { - reconnectFuture.cancel(true); - } + ExecutorUtil.cancelScheduledFuture(reconnectFuture); } catch (Throwable t) { logger.warn("Failed to cancel reconnect timer", t); } diff --git a/dubbo-registry/dubbo-registry-multicast/src/main/java/org/apache/dubbo/registry/multicast/MulticastRegistry.java b/dubbo-registry/dubbo-registry-multicast/src/main/java/org/apache/dubbo/registry/multicast/MulticastRegistry.java index 06835dac6e5..76b609d6345 100644 --- a/dubbo-registry/dubbo-registry-multicast/src/main/java/org/apache/dubbo/registry/multicast/MulticastRegistry.java +++ b/dubbo-registry/dubbo-registry-multicast/src/main/java/org/apache/dubbo/registry/multicast/MulticastRegistry.java @@ -297,9 +297,7 @@ public boolean isAvailable() { public void destroy() { super.destroy(); try { - if (cleanFuture != null) { - cleanFuture.cancel(true); - } + ExecutorUtil.cancelScheduledFuture(cleanFuture); } catch (Throwable t) { logger.warn(t.getMessage(), t); } @@ -341,8 +339,8 @@ protected void unregistered(URL url) { if (urls != null) { urls.remove(url); } - if (urls == null || urls.isEmpty()){ - if (urls == null){ + if (urls == null || urls.isEmpty()) { + if (urls == null) { urls = new ConcurrentHashSet(); } URL empty = url.setProtocol(Constants.EMPTY_PROTOCOL); diff --git a/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/exchange/support/FileExchangeGroup.java b/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/exchange/support/FileExchangeGroup.java index f2511a19abd..554b0e3dc2d 100644 --- a/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/exchange/support/FileExchangeGroup.java +++ b/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/exchange/support/FileExchangeGroup.java @@ -17,6 +17,7 @@ package org.apache.dubbo.remoting.p2p.exchange.support; import org.apache.dubbo.common.URL; +import org.apache.dubbo.common.utils.ExecutorUtil; import org.apache.dubbo.common.utils.IOUtils; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.common.utils.NetUtils; @@ -70,9 +71,7 @@ public void run() { public void close() { super.close(); try { - if (!checkModifiedFuture.isCancelled()) { - checkModifiedFuture.cancel(true); - } + ExecutorUtil.cancelScheduledFuture(checkModifiedFuture); } catch (Throwable t) { logger.error(t.getMessage(), t); } diff --git a/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/support/FileGroup.java b/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/support/FileGroup.java index 8a67e0edcc8..68562c15255 100644 --- a/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/support/FileGroup.java +++ b/dubbo-remoting/dubbo-remoting-p2p/src/main/java/org/apache/dubbo/remoting/p2p/support/FileGroup.java @@ -17,6 +17,7 @@ package org.apache.dubbo.remoting.p2p.support; import org.apache.dubbo.common.URL; +import org.apache.dubbo.common.utils.ExecutorUtil; import org.apache.dubbo.common.utils.IOUtils; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.common.utils.NetUtils; @@ -67,9 +68,7 @@ public void run() { public void close() { super.close(); try { - if (!checkModifiedFuture.isCancelled()) { - checkModifiedFuture.cancel(true); - } + ExecutorUtil.cancelScheduledFuture(checkModifiedFuture); } catch (Throwable t) { logger.error(t.getMessage(), t); } From 8dff2c122298fd303fe5221e0884cceff2c79c08 Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 8 Jan 2019 17:12:58 +0800 Subject: [PATCH 5/5] modify --- .../java/org/apache/dubbo/common/utils/ExecutorUtil.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java index c9f3e39e989..14408dacf64 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ExecutorUtil.java @@ -21,7 +21,12 @@ import org.apache.dubbo.common.logger.Logger; import org.apache.dubbo.common.logger.LoggerFactory; -import java.util.concurrent.*; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.ScheduledFuture; public class ExecutorUtil { private static final Logger logger = LoggerFactory.getLogger(ExecutorUtil.class);