From e4e0ad5807fed2437a697a1ba9d62c8e086c6c39 Mon Sep 17 00:00:00 2001 From: lizhen Date: Mon, 11 Feb 2019 16:14:07 +0800 Subject: [PATCH 1/3] Replace newSingleThreadScheduledExecutor with new Thread --- .../apache/dubbo/config/ServiceConfig.java | 38 +++++++++---------- .../dubbo/config/ServiceConfigTest.java | 21 +++++++++- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java index 3a85ac8b274..3297bd23539 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java @@ -27,7 +27,6 @@ import org.apache.dubbo.common.utils.ConfigUtils; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.common.utils.StringUtils; -import org.apache.dubbo.common.utils.CollectionUtils; import org.apache.dubbo.config.annotation.Service; import org.apache.dubbo.config.context.ConfigManager; import org.apache.dubbo.config.invoker.DelegateProviderMetaDataInvoker; @@ -56,8 +55,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import static org.apache.dubbo.common.Constants.LOCALHOST_VALUE; @@ -102,11 +99,6 @@ public class ServiceConfig extends AbstractServiceConfig { */ private static final Map RANDOM_PORT_MAP = new HashMap(); - /** - * A delayed exposure service timer - */ - private static final ScheduledExecutorService delayExportExecutor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("DubboServiceDelayExporter", true)); - /** * The urls of the services exported */ @@ -341,7 +333,15 @@ public synchronized void export() { } if (delay != null && delay > 0) { - delayExportExecutor.schedule(this::doExport, delay, TimeUnit.MILLISECONDS); + new NamedThreadFactory("DubboServiceDelayExporter", true).newThread(() -> { + try { + logger.info("Dubbo service " + interfaceClass.getName() + " will delay export for " + delay + " ms"); + TimeUnit.MILLISECONDS.sleep(delay); + doExport(); + } catch (InterruptedException e) { + logger.error("Expected error occured when export " + interfaceClass.getName(), e); + } + }).start(); } else { doExport(); } @@ -779,7 +779,7 @@ private void createProviderIfAbsent() { if (provider != null) { return; } - setProvider ( + setProvider( ConfigManager.getInstance() .getDefaultProvider() .orElseGet(() -> { @@ -810,15 +810,15 @@ private void convertProtocolIdsToProtocols() { if (StringUtils.isEmpty(protocolIds)) { if (CollectionUtils.isEmpty(protocols)) { - setProtocols( - ConfigManager.getInstance().getDefaultProtocols() - .filter(CollectionUtils::isNotEmpty) - .orElseGet(() -> { - ProtocolConfig protocolConfig = new ProtocolConfig(); - protocolConfig.refresh(); - return Arrays.asList(protocolConfig); - }) - ); + setProtocols( + ConfigManager.getInstance().getDefaultProtocols() + .filter(CollectionUtils::isNotEmpty) + .orElseGet(() -> { + ProtocolConfig protocolConfig = new ProtocolConfig(); + protocolConfig.refresh(); + return Arrays.asList(protocolConfig); + }) + ); } } else { String[] arr = Constants.COMMA_SPLIT_PATTERN.split(protocolIds); diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java index 1030bf35406..dd5c648d378 100644 --- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java +++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java @@ -31,7 +31,6 @@ import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.Protocol; import org.apache.dubbo.rpc.service.GenericService; - import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -40,6 +39,7 @@ import org.mockito.Mockito; import java.util.Collections; +import java.util.concurrent.TimeUnit; import static org.apache.dubbo.common.Constants.GENERIC_SERIALIZATION_BEAN; import static org.apache.dubbo.common.Constants.GENERIC_SERIALIZATION_DEFAULT; @@ -60,7 +60,7 @@ public class ServiceConfigTest { private Exporter exporter = Mockito.mock(Exporter.class); private ServiceConfig service = new ServiceConfig(); private ServiceConfig service2 = new ServiceConfig(); - + private ServiceConfig delayService = new ServiceConfig(); @BeforeEach public void setUp() throws Exception { @@ -104,6 +104,14 @@ public void setUp() throws Exception { service2.setMethods(Collections.singletonList(method)); service2.setProxy("testproxyfactory"); + delayService.setProvider(provider); + delayService.setApplication(app); + delayService.setRegistry(registry); + delayService.setInterface(DemoService.class); + delayService.setRef(new DemoServiceImpl()); + delayService.setMethods(Collections.singletonList(method)); + delayService.setDelay(100); + ConfigManager.getInstance().clear(); } @@ -143,6 +151,15 @@ public void testProxy() throws Exception { assertEquals(2, TestProxyFactory.count); // local injvm and registry protocol, so expected is 2 } + + @Test + public void testDelayExport() throws Exception { + delayService.export(); + //add 300ms to ensure that the delayService has been exported + TimeUnit.MILLISECONDS.sleep(delayService.getDelay() + 300); + assertThat(delayService.getExportedUrls(), hasSize(1)); + } + @Test @Disabled("cannot pass in travis") public void testUnexport() throws Exception { From 0d7bd2becdc4598f6f4a811e14123d4192ebd3c2 Mon Sep 17 00:00:00 2001 From: lizhen Date: Tue, 12 Feb 2019 14:57:58 +0800 Subject: [PATCH 2/3] Revert: Replace newSingleThreadScheduledExecutor with new Thread --- .../apache/dubbo/config/ServiceConfig.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java index 3297bd23539..3a85ac8b274 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java @@ -27,6 +27,7 @@ import org.apache.dubbo.common.utils.ConfigUtils; import org.apache.dubbo.common.utils.NamedThreadFactory; import org.apache.dubbo.common.utils.StringUtils; +import org.apache.dubbo.common.utils.CollectionUtils; import org.apache.dubbo.config.annotation.Service; import org.apache.dubbo.config.context.ConfigManager; import org.apache.dubbo.config.invoker.DelegateProviderMetaDataInvoker; @@ -55,6 +56,8 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import static org.apache.dubbo.common.Constants.LOCALHOST_VALUE; @@ -99,6 +102,11 @@ public class ServiceConfig extends AbstractServiceConfig { */ private static final Map RANDOM_PORT_MAP = new HashMap(); + /** + * A delayed exposure service timer + */ + private static final ScheduledExecutorService delayExportExecutor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("DubboServiceDelayExporter", true)); + /** * The urls of the services exported */ @@ -333,15 +341,7 @@ public synchronized void export() { } if (delay != null && delay > 0) { - new NamedThreadFactory("DubboServiceDelayExporter", true).newThread(() -> { - try { - logger.info("Dubbo service " + interfaceClass.getName() + " will delay export for " + delay + " ms"); - TimeUnit.MILLISECONDS.sleep(delay); - doExport(); - } catch (InterruptedException e) { - logger.error("Expected error occured when export " + interfaceClass.getName(), e); - } - }).start(); + delayExportExecutor.schedule(this::doExport, delay, TimeUnit.MILLISECONDS); } else { doExport(); } @@ -779,7 +779,7 @@ private void createProviderIfAbsent() { if (provider != null) { return; } - setProvider( + setProvider ( ConfigManager.getInstance() .getDefaultProvider() .orElseGet(() -> { @@ -810,15 +810,15 @@ private void convertProtocolIdsToProtocols() { if (StringUtils.isEmpty(protocolIds)) { if (CollectionUtils.isEmpty(protocols)) { - setProtocols( - ConfigManager.getInstance().getDefaultProtocols() - .filter(CollectionUtils::isNotEmpty) - .orElseGet(() -> { - ProtocolConfig protocolConfig = new ProtocolConfig(); - protocolConfig.refresh(); - return Arrays.asList(protocolConfig); - }) - ); + setProtocols( + ConfigManager.getInstance().getDefaultProtocols() + .filter(CollectionUtils::isNotEmpty) + .orElseGet(() -> { + ProtocolConfig protocolConfig = new ProtocolConfig(); + protocolConfig.refresh(); + return Arrays.asList(protocolConfig); + }) + ); } } else { String[] arr = Constants.COMMA_SPLIT_PATTERN.split(protocolIds); From 1590377a4542cbe0d40d4fa1b5d594467b5c9773 Mon Sep 17 00:00:00 2001 From: lizhen Date: Tue, 12 Feb 2019 15:32:38 +0800 Subject: [PATCH 3/3] add assert --- .../test/java/org/apache/dubbo/config/ServiceConfigTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java index dd5c648d378..dc3d253a937 100644 --- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java +++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/ServiceConfigTest.java @@ -52,6 +52,7 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.withSettings; public class ServiceConfigTest { @@ -155,6 +156,7 @@ public void testProxy() throws Exception { @Test public void testDelayExport() throws Exception { delayService.export(); + assertTrue(delayService.getExportedUrls().isEmpty()); //add 300ms to ensure that the delayService has been exported TimeUnit.MILLISECONDS.sleep(delayService.getDelay() + 300); assertThat(delayService.getExportedUrls(), hasSize(1));