From 740be9ca0d9a3d44e0ce9ab5c26e173305b5ae97 Mon Sep 17 00:00:00 2001 From: zonghaishang Date: Mon, 7 Jan 2019 16:04:59 +0800 Subject: [PATCH 1/4] fix client reconnect offline provider. --- .../remoting/transport/AbstractClient.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java index ed913d2a0f7..4c052f92d6b 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java @@ -152,6 +152,25 @@ private synchronized void initConnectStatusCheckCommand() { @Override public void run() { try { + + /** + * If the provider service is detected offline, + * the client should not attempt to connect again. + * + * issue: https://github.com/apache/incubator-dubbo/issues/3158 + */ + if(isClosed()) { + ScheduledFuture future = reconnectExecutorFuture; + if(future != null && !future.isCancelled()){ + /** + * Client has been destroyed and + * scheduled task should be cancelled. + */ + future.cancel(true); + } + return; + } + if (!isConnected()) { connect(); } else { @@ -344,14 +363,14 @@ public void reconnect() throws RemotingException { @Override public void close() { try { - if (executor != null) { - ExecutorUtil.shutdownNow(executor, 100); - } + super.close(); } catch (Throwable e) { logger.warn(e.getMessage(), e); } try { - super.close(); + if (executor != null) { + ExecutorUtil.shutdownNow(executor, 100); + } } catch (Throwable e) { logger.warn(e.getMessage(), e); } From 5fdd75b6932f8192bb2bc0ead13e54a4e0acc5c2 Mon Sep 17 00:00:00 2001 From: zonghaishang Date: Tue, 8 Jan 2019 00:12:30 +0800 Subject: [PATCH 2/4] refactor cancel future. --- .../remoting/transport/AbstractClient.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java index 4c052f92d6b..cfb3772d79e 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java @@ -152,24 +152,7 @@ private synchronized void initConnectStatusCheckCommand() { @Override public void run() { try { - - /** - * If the provider service is detected offline, - * the client should not attempt to connect again. - * - * issue: https://github.com/apache/incubator-dubbo/issues/3158 - */ - if(isClosed()) { - ScheduledFuture future = reconnectExecutorFuture; - if(future != null && !future.isCancelled()){ - /** - * Client has been destroyed and - * scheduled task should be cancelled. - */ - future.cancel(true); - } - return; - } + if (cancelFutureIfOffline()) return; if (!isConnected()) { connect(); @@ -191,7 +174,29 @@ public void run() { } } } + + private boolean cancelFutureIfOffline() { + /** + * If the provider service is detected offline, + * the client should not attempt to connect again. + * + * issue: https://github.com/apache/incubator-dubbo/issues/3158 + */ + if(isClosed()) { + ScheduledFuture future = reconnectExecutorFuture; + if(future != null && !future.isCancelled()){ + /** + * Client has been destroyed and + * scheduled task should be cancelled. + */ + future.cancel(true); + } + return true; + } + return false; + } }; + reconnectExecutorFuture = reconnectExecutorService.scheduleWithFixedDelay(connectStatusCheckCommand, reconnect, reconnect, TimeUnit.MILLISECONDS); } } From 2c62788f223c1e2f965c9a5c4533e7a6505f7868 Mon Sep 17 00:00:00 2001 From: zonghaishang Date: Thu, 10 Jan 2019 14:14:54 +0800 Subject: [PATCH 3/4] add comments --- .../registry/integration/RegistryDirectory.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java index 5ebace2a4b2..0b50c35dfcf 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java @@ -216,9 +216,7 @@ private void refreshOverrideAndInvoker(List urls) { private void refreshInvoker(List invokerUrls) { Assert.notNull(invokerUrls, "invokerUrls should not be null"); - if (invokerUrls.size() == 1 && invokerUrls.get(0) != null && Constants.EMPTY_PROTOCOL.equals(invokerUrls - .get(0) - .getProtocol())) { + if (invokerUrls.size() == 1 && Constants.EMPTY_PROTOCOL.equals(invokerUrls.get(0).getProtocol())) { this.forbidden = true; // Forbid to access this.invokers = Collections.emptyList(); routerChain.setInvokers(this.invokers); @@ -240,8 +238,14 @@ private void refreshInvoker(List invokerUrls) { } Map> newUrlInvokerMap = toInvokers(invokerUrls);// Translate url list to Invoker map - // state change - // If the calculation is wrong, it is not processed. + /** + * If the calculation is wrong, it is not processed. + * + * 1. The protocol configured by the client is inconsistent with the protocol of the server. + * eg: consumer protocol = dubbo, provider only has other protocol services(rest). + * 2. The registration center is not robust and pushes illegal specification data. + * + */ if (newUrlInvokerMap == null || newUrlInvokerMap.size() == 0) { logger.error(new IllegalStateException("urls to invokers error .invokerUrls.size :" + invokerUrls.size() + ", invoker.size :0. urls :" + invokerUrls .toString())); From 0bc0097b2e5534dce3b6d2b87d74ad7cf56aa6f4 Mon Sep 17 00:00:00 2001 From: "ken.lj" Date: Wed, 23 Jan 2019 17:20:23 +0800 Subject: [PATCH 4/4] polish code add {} for if --- .../org/apache/dubbo/remoting/transport/AbstractClient.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java index 7280b508f5e..96a2e9976e4 100644 --- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java +++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java @@ -153,7 +153,9 @@ private synchronized void initConnectStatusCheckCommand() { @Override public void run() { try { - if (cancelFutureIfOffline()) return; + if (cancelFutureIfOffline()) { + return; + } if (!isConnected()) { connect();