From f9b27eddef0ea40f17314320ce38f4368e7ca1ae Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sat, 14 Jan 2023 14:32:48 +0800 Subject: [PATCH 01/11] Fix service discovery metadata update failed (#11298) --- .../registry/client/AbstractServiceDiscovery.java | 14 ++++++++++++-- .../registry/nacos/NacosServiceDiscovery.java | 4 +++- .../zookeeper/ZookeeperServiceDiscovery.java | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java index 77f983a743..d3431d77e9 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java @@ -296,12 +296,22 @@ public abstract class AbstractServiceDiscovery implements ServiceDiscovery { throw new UnsupportedOperationException("Service discovery implementation does not support lookup of url list."); } - protected void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException { + /** + * Update Service Instance. Unregister and then register by default. + * Can be override if registry support update instance directly. + *
+ * NOTICE: Remind to update {@link AbstractServiceDiscovery#serviceInstance}'s reference if updated + * and report metadata by {@link AbstractServiceDiscovery#reportMetadata(MetadataInfo)} + * + * @param oldServiceInstance origin service instance + * @param newServiceInstance new service instance + */ + protected void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) { this.doUnregister(oldServiceInstance); if (!EMPTY_REVISION.equals(getExportedServicesRevision(serviceInstance))) { - reportMetadata(serviceInstance.getServiceMetadata()); this.serviceInstance = newServiceInstance; + reportMetadata(newServiceInstance.getServiceMetadata()); this.doRegister(newServiceInstance); } } diff --git a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java index 28c2efd44e..a589ec19ee 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java +++ b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java @@ -113,13 +113,15 @@ public class NacosServiceDiscovery extends AbstractServiceDiscovery { if (!Objects.equals(oldServiceInstance.getServiceName(), newServiceInstance.getServiceName()) || !Objects.equals(oldServiceInstance.getAddress(), newServiceInstance.getAddress()) || !Objects.equals(oldServiceInstance.getPort(), newServiceInstance.getPort())) { - // ignore if host-ip changed + // Ignore if host-ip changed. Should unregister first. super.doUpdate(oldServiceInstance, newServiceInstance); return; } try { this.serviceInstance = newServiceInstance; + reportMetadata(newServiceInstance.getServiceMetadata()); + // override without unregister this.doRegister(newServiceInstance); } catch (Exception e) { diff --git a/dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java b/dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java index a4897bfc88..a27675b5c4 100644 --- a/dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java +++ b/dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java @@ -120,7 +120,7 @@ public class ZookeeperServiceDiscovery extends AbstractServiceDiscovery { org.apache.curator.x.discovery.ServiceInstance newInstance = build(newServiceInstance); if (!Objects.equals(newInstance.getName(), oldInstance.getName()) || !Objects.equals(newInstance.getId(), oldInstance.getId())) { - // ignore if id changed + // Ignore if id changed. Should unregister first. super.doUpdate(oldServiceInstance, newServiceInstance); return; } From 673375b9bf3200b23b52aaa84eab61ee148d6f6e Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sat, 14 Jan 2023 21:41:07 +0800 Subject: [PATCH 02/11] Add nacos create client retry (#11304) --- .../nacos/NacosDynamicConfiguration.java | 59 ++++++++++--- .../support/nacos/MockConfigService.java | 78 +++++++++++++++++ .../configcenter/support/nacos/RetryTest.java | 86 +++++++++++++++++++ .../store/nacos/NacosMetadataReport.java | 73 ++++++++++++---- .../store/nacos/MockConfigService.java | 78 +++++++++++++++++ .../dubbo/metadata/store/nacos/RetryTest.java | 85 ++++++++++++++++++ .../nacos/NacosNamingServiceWrapper.java | 4 +- .../nacos/util/NacosNamingServiceUtils.java | 41 +++++++-- .../nacos/NacosRegistryFactoryTest.java | 5 +- .../registry/nacos/NacosRegistryTest.java | 2 +- .../NacosServiceDiscoveryFactoryTest.java | 3 +- .../nacos/NacosServiceDiscoveryTest.java | 26 +++--- .../util/NacosNamingServiceUtilsTest.java | 47 ++++++++-- 13 files changed, 523 insertions(+), 64 deletions(-) create mode 100644 dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/MockConfigService.java create mode 100644 dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java create mode 100644 dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/MockConfigService.java create mode 100644 dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java diff --git a/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java b/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java index 2f43c85f27..beb2400860 100644 --- a/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java +++ b/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java @@ -17,12 +17,20 @@ package org.apache.dubbo.configcenter.support.nacos; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.Executor; + import org.apache.dubbo.common.URL; import org.apache.dubbo.common.config.configcenter.ConfigChangeType; import org.apache.dubbo.common.config.configcenter.ConfigChangedEvent; import org.apache.dubbo.common.config.configcenter.ConfigItem; import org.apache.dubbo.common.config.configcenter.ConfigurationListener; import org.apache.dubbo.common.config.configcenter.DynamicConfiguration; +import org.apache.dubbo.common.constants.LoggerCodeConstants; import org.apache.dubbo.common.logger.ErrorTypeAwareLogger; import org.apache.dubbo.common.logger.LoggerFactory; import org.apache.dubbo.common.utils.MD5Utils; @@ -34,17 +42,12 @@ import com.alibaba.nacos.api.config.ConfigService; import com.alibaba.nacos.api.config.listener.AbstractSharedListener; import com.alibaba.nacos.api.exception.NacosException; -import java.util.Map; -import java.util.Properties; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.Executor; - import static com.alibaba.nacos.api.PropertyKeyConst.PASSWORD; import static com.alibaba.nacos.api.PropertyKeyConst.SERVER_ADDR; import static com.alibaba.nacos.api.PropertyKeyConst.USERNAME; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; import static org.apache.dubbo.common.constants.LoggerCodeConstants.CONFIG_ERROR_NACOS; +import static org.apache.dubbo.common.constants.LoggerCodeConstants.INTERNAL_INTERRUPTED; import static org.apache.dubbo.common.constants.RemotingConstants.BACKUP_KEY; import static org.apache.dubbo.common.utils.StringConstantFieldValuePredicate.of; import static org.apache.dubbo.common.utils.StringUtils.HYPHEN_CHAR; @@ -64,6 +67,12 @@ public class NacosDynamicConfiguration implements DynamicConfiguration { private Properties nacosProperties; + private static final String NACOS_RETRY_KEY = "nacos.retry"; + + private static final String NACOS_RETRY_WAIT_KEY = "nacos.retry-wait"; + + private static final String NACOS_CHECK_KEY = "nacos.check"; + /** * The nacos configService */ @@ -83,16 +92,40 @@ public class NacosDynamicConfiguration implements DynamicConfiguration { } private NacosConfigServiceWrapper buildConfigService(URL url) { - ConfigService configService = null; + int retryTimes = url.getPositiveParameter(NACOS_RETRY_KEY, 10); + int sleepMsBetweenRetries = url.getPositiveParameter(NACOS_RETRY_WAIT_KEY, 1000); + boolean check = url.getParameter(NACOS_CHECK_KEY, true); + ConfigService tmpConfigServices = null; try { - configService = NacosFactory.createConfigService(nacosProperties); - } catch (NacosException e) { - if (logger.isErrorEnabled()) { - logger.error(CONFIG_ERROR_NACOS, "", "", e.getMessage(), e); + for (int i = 0; i < retryTimes + 1; i++) { + tmpConfigServices = NacosFactory.createConfigService(nacosProperties); + if (!check || UP.equals(tmpConfigServices.getServerStatus())) { + break; + } else { + logger.warn(LoggerCodeConstants.CONFIG_ERROR_NACOS, "", "", + "Failed to connect to nacos config server. " + + (i < retryTimes ? "Dubbo will try to retry in " + sleepMsBetweenRetries + ". " : "Exceed retry max times.") + + "Try times: " + (i + 1)); + } + tmpConfigServices.shutDown(); + tmpConfigServices = null; + Thread.sleep(sleepMsBetweenRetries); } + } catch (NacosException e) { + logger.error(CONFIG_ERROR_NACOS, "", "", e.getErrMsg(), e); + throw new IllegalStateException(e); + } catch (InterruptedException e) { + logger.error(INTERNAL_INTERRUPTED, "", "", "Interrupted when creating nacos config service client.", e); + Thread.currentThread().interrupt(); throw new IllegalStateException(e); } - return new NacosConfigServiceWrapper(configService); + + if (tmpConfigServices == null) { + logger.error(CONFIG_ERROR_NACOS, "", "", "Failed to create nacos config service client. Reason: server status check failed."); + throw new IllegalStateException("Failed to create nacos config service client. Reason: server status check failed."); + } + + return new NacosConfigServiceWrapper(tmpConfigServices); } private Properties buildNacosProperties(URL url) { diff --git a/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/MockConfigService.java b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/MockConfigService.java new file mode 100644 index 0000000000..bba7741e64 --- /dev/null +++ b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/MockConfigService.java @@ -0,0 +1,78 @@ +/* + * 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.configcenter.support.nacos; + +import com.alibaba.nacos.api.config.ConfigService; +import com.alibaba.nacos.api.config.listener.Listener; +import com.alibaba.nacos.api.exception.NacosException; + +public class MockConfigService implements ConfigService { + @Override + public String getConfig(String dataId, String group, long timeoutMs) throws NacosException { + return null; + } + + @Override + public String getConfigAndSignListener(String dataId, String group, long timeoutMs, Listener listener) throws NacosException { + return null; + } + + @Override + public void addListener(String dataId, String group, Listener listener) throws NacosException { + + } + + @Override + public boolean publishConfig(String dataId, String group, String content) throws NacosException { + return false; + } + + @Override + public boolean publishConfig(String dataId, String group, String content, String type) throws NacosException { + return false; + } + + @Override + public boolean publishConfigCas(String dataId, String group, String content, String casMd5) throws NacosException { + return false; + } + + @Override + public boolean publishConfigCas(String dataId, String group, String content, String casMd5, String type) throws NacosException { + return false; + } + + @Override + public boolean removeConfig(String dataId, String group) throws NacosException { + return false; + } + + @Override + public void removeListener(String dataId, String group, Listener listener) { + + } + + @Override + public String getServerStatus() { + return null; + } + + @Override + public void shutDown() throws NacosException { + + } +} diff --git a/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java new file mode 100644 index 0000000000..e90a5cb016 --- /dev/null +++ b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java @@ -0,0 +1,86 @@ +/* + * 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.configcenter.support.nacos; + +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.dubbo.common.URL; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import com.alibaba.nacos.api.NacosFactory; +import com.alibaba.nacos.api.config.ConfigService; + +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.DOWN; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; +import static org.mockito.ArgumentMatchers.any; + +class RetryTest { + + @Test + void testRetryCreate() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + ConfigService mock = new MockConfigService() { + @Override + public String getServerStatus() { + return atomicInteger.incrementAndGet() > 10 ? UP : DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> new NacosDynamicConfiguration(url)); + + try { + new NacosDynamicConfiguration(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } + + @Test + void testDisable() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + ConfigService mock = new MockConfigService() { + @Override + public String getServerStatus() { + return DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10) + .addParameter("nacos.check", "false"); + try { + new NacosDynamicConfiguration(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } +} diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java index 88f65ba642..9d2a93c24a 100644 --- a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java @@ -17,11 +17,24 @@ package org.apache.dubbo.metadata.store.nacos; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.Executor; + import org.apache.dubbo.common.URL; import org.apache.dubbo.common.config.configcenter.ConfigChangeType; import org.apache.dubbo.common.config.configcenter.ConfigChangedEvent; import org.apache.dubbo.common.config.configcenter.ConfigItem; import org.apache.dubbo.common.config.configcenter.ConfigurationListener; +import org.apache.dubbo.common.constants.LoggerCodeConstants; import org.apache.dubbo.common.utils.JsonUtils; import org.apache.dubbo.common.utils.MD5Utils; import org.apache.dubbo.common.utils.StringUtils; @@ -38,23 +51,15 @@ import org.apache.dubbo.metadata.report.support.AbstractMetadataReport; import com.alibaba.nacos.api.NacosFactory; import com.alibaba.nacos.api.PropertyKeyConst; +import com.alibaba.nacos.api.config.ConfigService; import com.alibaba.nacos.api.config.listener.AbstractSharedListener; import com.alibaba.nacos.api.exception.NacosException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.Executor; - import static com.alibaba.nacos.api.PropertyKeyConst.SERVER_ADDR; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; import static org.apache.dubbo.common.constants.CommonConstants.GROUP_KEY; +import static org.apache.dubbo.common.constants.LoggerCodeConstants.CONFIG_ERROR_NACOS; +import static org.apache.dubbo.common.constants.LoggerCodeConstants.INTERNAL_INTERRUPTED; import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION; import static org.apache.dubbo.common.constants.RemotingConstants.BACKUP_KEY; import static org.apache.dubbo.common.utils.StringConstantFieldValuePredicate.of; @@ -81,25 +86,57 @@ public class NacosMetadataReport extends AbstractMetadataReport { private MD5Utils md5Utils = new MD5Utils(); + private static final String NACOS_RETRY_KEY = "nacos.retry"; + + private static final String NACOS_RETRY_WAIT_KEY = "nacos.retry-wait"; + + private static final String NACOS_CHECK_KEY = "nacos.check"; + public NacosMetadataReport(URL url) { super(url); this.configService = buildConfigService(url); group = url.getParameter(GROUP_KEY, DEFAULT_ROOT); } - public NacosConfigServiceWrapper buildConfigService(URL url) { + private NacosConfigServiceWrapper buildConfigService(URL url) { Properties nacosProperties = buildNacosProperties(url); + int retryTimes = url.getPositiveParameter(NACOS_RETRY_KEY, 10); + int sleepMsBetweenRetries = url.getPositiveParameter(NACOS_RETRY_WAIT_KEY, 1000); + boolean check = url.getParameter(NACOS_CHECK_KEY, true); + ConfigService tmpConfigServices = null; try { - configService = new NacosConfigServiceWrapper(NacosFactory.createConfigService(nacosProperties)); - } catch (NacosException e) { - if (logger.isErrorEnabled()) { - logger.error(REGISTRY_NACOS_EXCEPTION, "", "", e.getErrMsg(), e); + for (int i = 0; i < retryTimes + 1; i++) { + tmpConfigServices = NacosFactory.createConfigService(nacosProperties); + if (!check || UP.equals(tmpConfigServices.getServerStatus())) { + break; + } else { + logger.warn(LoggerCodeConstants.CONFIG_ERROR_NACOS, "", "", + "Failed to connect to nacos config server. " + + (i < retryTimes ? "Dubbo will try to retry in " + sleepMsBetweenRetries + ". " : "Exceed retry max times.") + + "Try times: " + (i + 1)); + } + tmpConfigServices.shutDown(); + tmpConfigServices = null; + Thread.sleep(sleepMsBetweenRetries); } + } catch (NacosException e) { + logger.error(CONFIG_ERROR_NACOS, "", "", e.getErrMsg(), e); + throw new IllegalStateException(e); + } catch (InterruptedException e) { + logger.error(INTERNAL_INTERRUPTED, "", "", "Interrupted when creating nacos config service client.", e); + Thread.currentThread().interrupt(); throw new IllegalStateException(e); } - return configService; + + if (tmpConfigServices == null) { + logger.error(CONFIG_ERROR_NACOS, "", "", "Failed to create nacos config service client. Reason: server status check failed."); + throw new IllegalStateException("Failed to create nacos config service client. Reason: server status check failed."); + } + + return new NacosConfigServiceWrapper(tmpConfigServices); } + private Properties buildNacosProperties(URL url) { Properties properties = new Properties(); setServerAddr(url, properties); diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/MockConfigService.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/MockConfigService.java new file mode 100644 index 0000000000..86b5b879f7 --- /dev/null +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/MockConfigService.java @@ -0,0 +1,78 @@ +/* + * 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.metadata.store.nacos; + +import com.alibaba.nacos.api.config.ConfigService; +import com.alibaba.nacos.api.config.listener.Listener; +import com.alibaba.nacos.api.exception.NacosException; + +public class MockConfigService implements ConfigService { + @Override + public String getConfig(String dataId, String group, long timeoutMs) throws NacosException { + return null; + } + + @Override + public String getConfigAndSignListener(String dataId, String group, long timeoutMs, Listener listener) throws NacosException { + return null; + } + + @Override + public void addListener(String dataId, String group, Listener listener) throws NacosException { + + } + + @Override + public boolean publishConfig(String dataId, String group, String content) throws NacosException { + return false; + } + + @Override + public boolean publishConfig(String dataId, String group, String content, String type) throws NacosException { + return false; + } + + @Override + public boolean publishConfigCas(String dataId, String group, String content, String casMd5) throws NacosException { + return false; + } + + @Override + public boolean publishConfigCas(String dataId, String group, String content, String casMd5, String type) throws NacosException { + return false; + } + + @Override + public boolean removeConfig(String dataId, String group) throws NacosException { + return false; + } + + @Override + public void removeListener(String dataId, String group, Listener listener) { + + } + + @Override + public String getServerStatus() { + return null; + } + + @Override + public void shutDown() throws NacosException { + + } +} diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java new file mode 100644 index 0000000000..9159e076fc --- /dev/null +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java @@ -0,0 +1,85 @@ +/* + * 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.metadata.store.nacos; + +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.dubbo.common.URL; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import com.alibaba.nacos.api.NacosFactory; +import com.alibaba.nacos.api.config.ConfigService; + +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.DOWN; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; +import static org.mockito.ArgumentMatchers.any; + +class RetryTest { + + @Test + void testRetryCreate() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + ConfigService mock = new MockConfigService() { + @Override + public String getServerStatus() { + return atomicInteger.incrementAndGet() > 10 ? UP : DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> new NacosMetadataReport(url)); + + try { + new NacosMetadataReport(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } + @Test + void testDisable() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + ConfigService mock = new MockConfigService() { + @Override + public String getServerStatus() { + return DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10) + .addParameter("nacos.check", "false"); + try { + new NacosMetadataReport(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } +} diff --git a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java index e1185aa15b..6bc12e17b6 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java +++ b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java @@ -117,7 +117,7 @@ public class NacosNamingServiceWrapper { logger.warn(LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION, "", "", "Failed to request nacos naming server. " + (times < retryTimes ? "Dubbo will try to retry in " + sleepMsBetweenRetries + ". " : "Exceed retry max times.") + - "Try times: " + times + 1, e); + "Try times: " + (times + 1), e); if (times < retryTimes) { try { Thread.sleep(sleepMsBetweenRetries); @@ -151,7 +151,7 @@ public class NacosNamingServiceWrapper { logger.warn(LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION, "", "", "Failed to request nacos naming server. " + (times < retryTimes ? "Dubbo will try to retry in " + sleepMsBetweenRetries + ". " : "Exceed retry max times.") + - "Try times: " + times + 1, e); + "Try times: " + (times + 1), e); if (times < retryTimes) { try { Thread.sleep(sleepMsBetweenRetries); diff --git a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java index c085701d86..b7809911a2 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java +++ b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.Properties; import org.apache.dubbo.common.URL; +import org.apache.dubbo.common.constants.LoggerCodeConstants; import org.apache.dubbo.common.logger.ErrorTypeAwareLogger; import org.apache.dubbo.common.logger.LoggerFactory; import org.apache.dubbo.common.utils.StringUtils; @@ -39,8 +40,10 @@ import static com.alibaba.nacos.api.PropertyKeyConst.PASSWORD; import static com.alibaba.nacos.api.PropertyKeyConst.SERVER_ADDR; import static com.alibaba.nacos.api.PropertyKeyConst.USERNAME; import static com.alibaba.nacos.api.common.Constants.DEFAULT_GROUP; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; import static com.alibaba.nacos.client.naming.utils.UtilAndComs.NACOS_NAMING_LOG_NAME; import static org.apache.dubbo.common.constants.CommonConstants.GROUP_KEY; +import static org.apache.dubbo.common.constants.LoggerCodeConstants.INTERNAL_INTERRUPTED; import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION; import static org.apache.dubbo.common.constants.RemotingConstants.BACKUP_KEY; import static org.apache.dubbo.common.utils.StringConstantFieldValuePredicate.of; @@ -57,6 +60,8 @@ public class NacosNamingServiceUtils { private static final String NACOS_RETRY_KEY = "nacos.retry"; + private static final String NACOS_CHECK_KEY = "nacos.check"; + private static final String NACOS_RETRY_WAIT_KEY = "nacos.retry-wait"; @@ -119,17 +124,39 @@ public class NacosNamingServiceUtils { */ public static NacosNamingServiceWrapper createNamingService(URL connectionURL) { Properties nacosProperties = buildNacosProperties(connectionURL); - NamingService namingService; + int retryTimes = connectionURL.getPositiveParameter(NACOS_RETRY_KEY, 10); + int sleepMsBetweenRetries = connectionURL.getPositiveParameter(NACOS_RETRY_WAIT_KEY, 1000); + boolean check = connectionURL.getParameter(NACOS_CHECK_KEY, true); + NamingService namingService = null; try { - namingService = NacosFactory.createNamingService(nacosProperties); - } catch (NacosException e) { - if (logger.isErrorEnabled()) { - logger.error(REGISTRY_NACOS_EXCEPTION, "", "", e.getErrMsg(), e); + for (int i = 0; i < retryTimes + 1; i++) { + namingService = NacosFactory.createNamingService(nacosProperties); + if (!check || UP.equals(namingService.getServerStatus())) { + break; + } else { + logger.warn(LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION, "", "", + "Failed to connect to nacos naming server. " + + (i < retryTimes ? "Dubbo will try to retry in " + sleepMsBetweenRetries + ". " : "Exceed retry max times.") + + "Try times: " + (i + 1)); + } + namingService.shutDown(); + namingService = null; + Thread.sleep(sleepMsBetweenRetries); } + } catch (NacosException e) { + logger.error(REGISTRY_NACOS_EXCEPTION, "", "", e.getErrMsg(), e); + throw new IllegalStateException(e); + } catch (InterruptedException e) { + logger.error(INTERNAL_INTERRUPTED, "", "", "Interrupted when creating nacos naming service client.", e); + Thread.currentThread().interrupt(); throw new IllegalStateException(e); } - int retryTimes = connectionURL.getParameter(NACOS_RETRY_KEY, 10); - int sleepMsBetweenRetries = connectionURL.getParameter(NACOS_RETRY_WAIT_KEY, 10); + + if (namingService == null) { + logger.error(REGISTRY_NACOS_EXCEPTION, "", "", "Failed to create nacos naming service client. Reason: server status check failed."); + throw new IllegalStateException("Failed to create nacos naming service client. Reason: server status check failed."); + } + return new NacosNamingServiceWrapper(namingService, retryTimes, sleepMsBetweenRetries); } diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryFactoryTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryFactoryTest.java index 1df7d4d760..2af57626a4 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryFactoryTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryFactoryTest.java @@ -18,7 +18,6 @@ package org.apache.dubbo.registry.nacos; import org.apache.dubbo.common.URL; import org.apache.dubbo.common.utils.NetUtils; - import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -42,7 +41,7 @@ class NacosRegistryFactoryTest { @Test void testCreateRegistryCacheKey() { - URL url = URL.valueOf("dubbo://" + NetUtils.getLocalAddress().getHostAddress() + ":8080"); + URL url = URL.valueOf("dubbo://" + NetUtils.getLocalAddress().getHostAddress() + ":8080?nacos.check=false"); String registryCacheKey1 = nacosRegistryFactory.createRegistryCacheKey(url); String registryCacheKey2 = nacosRegistryFactory.createRegistryCacheKey(url); Assertions.assertEquals(registryCacheKey1, registryCacheKey2); @@ -50,7 +49,7 @@ class NacosRegistryFactoryTest { @Test void testCreateRegistryCacheKeyWithNamespace() { - URL url = URL.valueOf("dubbo://" + NetUtils.getLocalAddress().getHostAddress() + ":8080?namespace=test"); + URL url = URL.valueOf("dubbo://" + NetUtils.getLocalAddress().getHostAddress() + ":8080?namespace=test&nacos.check=false"); String registryCacheKey1 = nacosRegistryFactory.createRegistryCacheKey(url); String registryCacheKey2 = nacosRegistryFactory.createRegistryCacheKey(url); Assertions.assertEquals(registryCacheKey1, registryCacheKey2); diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryTest.java index c0568d3c52..82efa81bd9 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosRegistryTest.java @@ -69,7 +69,7 @@ class NacosRegistryTest { int nacosServerPort = NetUtils.getAvailablePort(); - this.registryUrl = URL.valueOf("nacos://localhost:" + nacosServerPort); + this.registryUrl = URL.valueOf("nacos://localhost:" + nacosServerPort + "?nacos.check=false"); this.nacosRegistryFactory = new NacosRegistryFactory(); diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryFactoryTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryFactoryTest.java index c52466d9a6..6f9003490c 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryFactoryTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryFactoryTest.java @@ -20,7 +20,6 @@ import org.apache.dubbo.common.URL; import org.apache.dubbo.config.ApplicationConfig; import org.apache.dubbo.registry.client.ServiceDiscovery; import org.apache.dubbo.rpc.model.ApplicationModel; - import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -42,7 +41,7 @@ class NacosServiceDiscoveryFactoryTest { @Test void testGetServiceDiscoveryWithCache() { - URL url = URL.valueOf("dubbo://test:8080"); + URL url = URL.valueOf("dubbo://test:8080?nacos.check=false"); ServiceDiscovery discovery = nacosServiceDiscoveryFactory.createDiscovery(url); Assertions.assertTrue(discovery instanceof NacosServiceDiscovery); diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryTest.java index 962acca8af..d994a08f59 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/NacosServiceDiscoveryTest.java @@ -16,6 +16,12 @@ */ package org.apache.dubbo.registry.nacos; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; +import java.util.Set; + import org.apache.dubbo.common.URL; import org.apache.dubbo.common.utils.NetUtils; import org.apache.dubbo.config.ApplicationConfig; @@ -25,10 +31,6 @@ import org.apache.dubbo.registry.client.event.ServiceInstancesChangedEvent; import org.apache.dubbo.registry.client.event.listener.ServiceInstancesChangedListener; import org.apache.dubbo.rpc.model.ApplicationModel; import org.apache.dubbo.rpc.model.ScopeModelUtil; - -import com.alibaba.nacos.api.exception.NacosException; -import com.alibaba.nacos.api.naming.pojo.Instance; -import com.alibaba.nacos.api.naming.pojo.ListView; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -37,11 +39,9 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.internal.util.collections.Sets; -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.LinkedList; -import java.util.List; -import java.util.Set; +import com.alibaba.nacos.api.exception.NacosException; +import com.alibaba.nacos.api.naming.pojo.Instance; +import com.alibaba.nacos.api.naming.pojo.ListView; import static com.alibaba.nacos.api.common.Constants.DEFAULT_GROUP; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -63,7 +63,7 @@ class NacosServiceDiscoveryTest { private static final String LOCALHOST = "127.0.0.1"; - protected URL registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort()); + protected URL registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort() + "?nacos.check=false"); private NacosServiceDiscovery nacosServiceDiscovery; @@ -79,7 +79,7 @@ class NacosServiceDiscoveryTest { public NacosServiceDiscoveryGroupTest1() { super(); group = "test-group1"; - registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort()).addParameter("group", group); + registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort() + "?nacos.check=false").addParameter("group", group); } } @@ -87,7 +87,7 @@ class NacosServiceDiscoveryTest { public NacosServiceDiscoveryGroupTest2() { super(); group = "test-group2"; - registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort()).addParameter("group", group); + registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort() + "?nacos.check=false").addParameter("group", group); } } @@ -97,7 +97,7 @@ class NacosServiceDiscoveryTest { public NacosServiceDiscoveryGroupTest3() { super(); group = DEFAULT_GROUP; - registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort()).addParameter("group", "test-group3"); + registryUrl = URL.valueOf("nacos://127.0.0.1:" + NetUtils.getAvailablePort() + "?nacos.check=false").addParameter("group", "test-group3"); } @BeforeAll diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java index 701d97cffa..35c13d6a41 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java @@ -16,19 +16,29 @@ */ package org.apache.dubbo.registry.nacos.util; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; + import org.apache.dubbo.common.URL; import org.apache.dubbo.metadata.report.MetadataReport; import org.apache.dubbo.registry.client.ServiceInstance; +import org.apache.dubbo.registry.nacos.MockNamingService; import org.apache.dubbo.registry.nacos.NacosNamingServiceWrapper; - -import com.alibaba.nacos.api.naming.pojo.Instance; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import org.mockito.Mockito; -import java.util.HashMap; -import java.util.Map; +import com.alibaba.nacos.api.NacosFactory; +import com.alibaba.nacos.api.exception.NacosException; +import com.alibaba.nacos.api.naming.NamingService; +import com.alibaba.nacos.api.naming.pojo.Instance; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.DOWN; +import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; /** @@ -67,8 +77,35 @@ class NacosNamingServiceUtilsTest { @Test void testCreateNamingService() { - URL url = URL.valueOf("test://test:8080/test?backup=backup"); + URL url = URL.valueOf("test://test:8080/test?backup=backup&nacos.check=false"); NacosNamingServiceWrapper namingService = NacosNamingServiceUtils.createNamingService(url); Assertions.assertNotNull(namingService); } + + + @Test + void testRetryCreate() throws NacosException { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + NamingService mock = new MockNamingService() { + @Override + public String getServerStatus() { + return atomicInteger.incrementAndGet() > 10 ? UP : DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createNamingService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> NacosNamingServiceUtils.createNamingService(url)); + + try { + NacosNamingServiceUtils.createNamingService(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } } From 52d9c7b80bbf1d65f6a17bc8ee52eea8447dbd4f Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sat, 14 Jan 2023 22:08:21 +0800 Subject: [PATCH 03/11] Enable multi remote version integration test (#11306) --- .../build-and-test-scheduled-3.1.yml | 27 +++++++------- .../build-and-test-scheduled-3.2.yml | 24 +++++++------ ...{release-test-3.1.yml => release-test.yml} | 36 ++++++++++--------- 3 files changed, 48 insertions(+), 39 deletions(-) rename .github/workflows/{release-test-3.1.yml => release-test.yml} (96%) diff --git a/.github/workflows/build-and-test-scheduled-3.1.yml b/.github/workflows/build-and-test-scheduled-3.1.yml index f82c01fa3a..e73b013131 100644 --- a/.github/workflows/build-and-test-scheduled-3.1.yml +++ b/.github/workflows/build-and-test-scheduled-3.1.yml @@ -14,10 +14,10 @@ env: SHOW_ERROR_DETAIL: 1 #multi-version size limit VERSIONS_LIMIT: 4 + ALL_REMOTE_VERSION: true CANDIDATE_VERSIONS: ' - spring.version:4.3.30.RELEASE; - spring-boot.version:1.5.22.RELEASE; - spring-boot.version:2.4.1; + spring.version:5.3.24; + spring-boot.version:2.7.6; ' jobs: @@ -216,7 +216,7 @@ jobs: integration-test-prepare: runs-on: ubuntu-latest env: - JOB_COUNT: 3 + JOB_COUNT: 5 steps: - uses: actions/checkout@v3 with: @@ -237,12 +237,13 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} TEST_CASE_FILE: jobs/testjob_${{matrix.job_id}}.txt strategy: fail-fast: false matrix: - job_id: [1, 2, 3] + jdk: [ 8, 11, 17, 19 ] + job_id: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v3 with: @@ -269,10 +270,10 @@ jobs: with: name: test-list path: test/jobs/ - - name: "Set up JDK 8" + - name: "Set up JDK ${{matrix.jdk}}" uses: actions/setup-java@v1 with: - java-version: 8 + java-version: ${{matrix.jdk}} - name: "Init Candidate Versions" run: | DUBBO_VERSION="${{needs.build-source.outputs.version}}" @@ -287,7 +288,7 @@ jobs: if: always() uses: actions/upload-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/*-result* integration-test-result: @@ -295,7 +296,10 @@ jobs: if: always() runs-on: ubuntu-latest env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} + strategy: + matrix: + jdk: [ 8, 11, 17, 19 ] steps: - uses: actions/checkout@v3 with: @@ -304,12 +308,11 @@ jobs: - name: "Download test result" uses: actions/download-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/ - name: "Merge test result" run: ./test/scripts/merge-test-results.sh - error-code-inspecting: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/build-and-test-scheduled-3.2.yml b/.github/workflows/build-and-test-scheduled-3.2.yml index 02da5f7520..983c1b3530 100644 --- a/.github/workflows/build-and-test-scheduled-3.2.yml +++ b/.github/workflows/build-and-test-scheduled-3.2.yml @@ -14,10 +14,10 @@ env: SHOW_ERROR_DETAIL: 1 #multi-version size limit VERSIONS_LIMIT: 4 + ALL_REMOTE_VERSION: true CANDIDATE_VERSIONS: ' - spring.version:4.3.30.RELEASE; - spring-boot.version:1.5.22.RELEASE; - spring-boot.version:2.4.1; + spring.version:5.3.24; + spring-boot.version:2.7.6; ' jobs: @@ -216,7 +216,7 @@ jobs: integration-test-prepare: runs-on: ubuntu-latest env: - JOB_COUNT: 3 + JOB_COUNT: 5 steps: - uses: actions/checkout@v3 with: @@ -237,11 +237,12 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} TEST_CASE_FILE: jobs/testjob_${{matrix.job_id}}.txt strategy: fail-fast: false matrix: + jdk: [ 8, 11, 17, 19 ] job_id: [1, 2, 3] steps: - uses: actions/checkout@v3 @@ -269,10 +270,10 @@ jobs: with: name: test-list path: test/jobs/ - - name: "Set up JDK 8" + - name: "Set up JDK ${{matrix.jdk}}" uses: actions/setup-java@v1 with: - java-version: 8 + java-version: ${{matrix.jdk}} - name: "Init Candidate Versions" run: | DUBBO_VERSION="${{needs.build-source.outputs.version}}" @@ -287,7 +288,7 @@ jobs: if: always() uses: actions/upload-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/*-result* integration-test-result: @@ -295,7 +296,10 @@ jobs: if: always() runs-on: ubuntu-latest env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} + strategy: + matrix: + jdk: [ 8, 11, 17, 19 ] steps: - uses: actions/checkout@v3 with: @@ -304,7 +308,7 @@ jobs: - name: "Download test result" uses: actions/download-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/ - name: "Merge test result" run: ./test/scripts/merge-test-results.sh diff --git a/.github/workflows/release-test-3.1.yml b/.github/workflows/release-test.yml similarity index 96% rename from .github/workflows/release-test-3.1.yml rename to .github/workflows/release-test.yml index d5ef981f77..fd23403846 100644 --- a/.github/workflows/release-test-3.1.yml +++ b/.github/workflows/release-test.yml @@ -15,10 +15,10 @@ env: SHOW_ERROR_DETAIL: 1 #multi-version size limit VERSIONS_LIMIT: 4 + ALL_REMOTE_VERSION: true CANDIDATE_VERSIONS: ' - spring.version:4.3.30.RELEASE; - spring-boot.version:1.5.22.RELEASE; - spring-boot.version:2.4.1; + spring.version:5.3.24; + spring-boot.version:2.7.6; ' jobs: @@ -212,7 +212,7 @@ jobs: integration-test-prepare: runs-on: ubuntu-latest env: - JOB_COUNT: 3 + JOB_COUNT: 5 steps: - uses: actions/checkout@v3 with: @@ -222,7 +222,7 @@ jobs: run: | bash ./test/scripts/prepare-test.sh - name: "Upload test list" - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-list path: test/jobs @@ -233,11 +233,12 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} TEST_CASE_FILE: jobs/testjob_${{matrix.job_id}}.txt strategy: fail-fast: false matrix: + jdk: [ 8, 11, 17, 19 ] job_id: [1, 2, 3] steps: - uses: actions/checkout@v3 @@ -261,15 +262,14 @@ jobs: ${{ runner.os }}-dubbo-snapshot-${{ github.sha }} ${{ runner.os }}-dubbo-snapshot- - name: "Download test list" - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v3 with: name: test-list path: test/jobs/ - - name: "Set up JDK 8" - uses: actions/setup-java@v3 + - name: "Set up JDK ${{matrix.jdk}}" + uses: actions/setup-java@v1 with: - java-version: 8 - distribution: 'zulu' + java-version: ${{matrix.jdk}} - name: "Init Candidate Versions" run: | DUBBO_VERSION="${{needs.build-source.outputs.version}}" @@ -282,9 +282,9 @@ jobs: run: cd test && bash ./run-tests.sh - name: "Upload test result" if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/*-result* integration-test-result: @@ -292,21 +292,23 @@ jobs: if: always() runs-on: ubuntu-latest env: - JAVA_VER: 8 + JAVA_VER: ${{matrix.jdk}} + strategy: + matrix: + jdk: [ 8, 11, 17, 19 ] steps: - uses: actions/checkout@v3 with: repository: 'apache/dubbo-samples' ref: master - name: "Download test result" - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v3 with: - name: test-result + name: test-result-${{matrix.jdk}} path: test/jobs/ - name: "Merge test result" run: ./test/scripts/merge-test-results.sh - error-code-inspecting: runs-on: ubuntu-latest steps: From 007b6bfd0b4ffca7c5e848bc68e50b3bd53546de Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Sat, 14 Jan 2023 22:19:26 +0800 Subject: [PATCH 04/11] Fix ci matrix --- .github/workflows/build-and-test-scheduled-3.2.yml | 2 +- .github/workflows/release-test.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-and-test-scheduled-3.2.yml b/.github/workflows/build-and-test-scheduled-3.2.yml index 983c1b3530..e28282881b 100644 --- a/.github/workflows/build-and-test-scheduled-3.2.yml +++ b/.github/workflows/build-and-test-scheduled-3.2.yml @@ -243,7 +243,7 @@ jobs: fail-fast: false matrix: jdk: [ 8, 11, 17, 19 ] - job_id: [1, 2, 3] + job_id: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v3 with: diff --git a/.github/workflows/release-test.yml b/.github/workflows/release-test.yml index fd23403846..1f15b5a62d 100644 --- a/.github/workflows/release-test.yml +++ b/.github/workflows/release-test.yml @@ -239,7 +239,7 @@ jobs: fail-fast: false matrix: jdk: [ 8, 11, 17, 19 ] - job_id: [1, 2, 3] + job_id: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v3 with: From e0971edcd98971c0ccc1da4164ce451448d11093 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Mon, 16 Jan 2023 10:22:19 +0800 Subject: [PATCH 05/11] Update fastjson2 version (#11305) --- dubbo-dependencies-bom/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dubbo-dependencies-bom/pom.xml b/dubbo-dependencies-bom/pom.xml index 53d6e84c1d..de0bb96cea 100644 --- a/dubbo-dependencies-bom/pom.xml +++ b/dubbo-dependencies-bom/pom.xml @@ -100,7 +100,7 @@ 4.5.13 4.4.6 1.2.83 - 2.0.21 + 2.0.23 3.4.14 4.2.0 2.12.0 From 486b39f28d4ca30005c15e1fde852ec057a1148d Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Mon, 16 Jan 2023 11:07:20 +0800 Subject: [PATCH 06/11] Add Nacos sub try test (#11307) --- .../nacos/NacosDynamicConfiguration.java | 11 +++- .../configcenter/support/nacos/RetryTest.java | 36 +++++++++++ .../store/nacos/NacosMetadataReport.java | 10 +++- .../dubbo/metadata/store/nacos/RetryTest.java | 36 +++++++++++ .../nacos/util/NacosNamingServiceUtils.java | 11 +++- .../util/NacosNamingServiceUtilsTest.java | 60 +++++++++++++++++++ 6 files changed, 161 insertions(+), 3 deletions(-) diff --git a/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java b/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java index beb2400860..5a09e03d06 100644 --- a/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java +++ b/dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java @@ -99,7 +99,7 @@ public class NacosDynamicConfiguration implements DynamicConfiguration { try { for (int i = 0; i < retryTimes + 1; i++) { tmpConfigServices = NacosFactory.createConfigService(nacosProperties); - if (!check || UP.equals(tmpConfigServices.getServerStatus())) { + if (!check || (UP.equals(tmpConfigServices.getServerStatus()) && testConfigService(tmpConfigServices))) { break; } else { logger.warn(LoggerCodeConstants.CONFIG_ERROR_NACOS, "", "", @@ -128,6 +128,15 @@ public class NacosDynamicConfiguration implements DynamicConfiguration { return new NacosConfigServiceWrapper(tmpConfigServices); } + private boolean testConfigService(ConfigService configService) { + try { + configService.getConfig("Dubbo-Nacos-Test", "Dubbo-Nacos-Test", DEFAULT_TIMEOUT); + return true; + } catch (NacosException e) { + return false; + } + } + private Properties buildNacosProperties(URL url) { Properties properties = new Properties(); setServerAddr(url, properties); diff --git a/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java index e90a5cb016..3be9037380 100644 --- a/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java +++ b/dubbo-configcenter/dubbo-configcenter-nacos/src/test/java/org/apache/dubbo/configcenter/support/nacos/RetryTest.java @@ -27,6 +27,7 @@ import org.mockito.Mockito; import com.alibaba.nacos.api.NacosFactory; import com.alibaba.nacos.api.config.ConfigService; +import com.alibaba.nacos.api.exception.NacosException; import static com.alibaba.nacos.client.constant.Constants.HealthCheck.DOWN; import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; @@ -83,4 +84,39 @@ class RetryTest { } } } + + @Test + void testRequest() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + ConfigService mock = new MockConfigService() { + @Override + public String getConfig(String dataId, String group, long timeoutMs) throws NacosException { + if (atomicInteger.incrementAndGet() > 10) { + return ""; + } else { + throw new NacosException(); + } + } + + @Override + public String getServerStatus() { + return UP; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> new NacosDynamicConfiguration(url)); + + try { + new NacosDynamicConfiguration(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } } diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java index 9d2a93c24a..70a9a1210c 100644 --- a/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/main/java/org/apache/dubbo/metadata/store/nacos/NacosMetadataReport.java @@ -107,7 +107,7 @@ public class NacosMetadataReport extends AbstractMetadataReport { try { for (int i = 0; i < retryTimes + 1; i++) { tmpConfigServices = NacosFactory.createConfigService(nacosProperties); - if (!check || UP.equals(tmpConfigServices.getServerStatus())) { + if (!check || (UP.equals(tmpConfigServices.getServerStatus()) && testConfigService(tmpConfigServices))) { break; } else { logger.warn(LoggerCodeConstants.CONFIG_ERROR_NACOS, "", "", @@ -136,6 +136,14 @@ public class NacosMetadataReport extends AbstractMetadataReport { return new NacosConfigServiceWrapper(tmpConfigServices); } + private boolean testConfigService(ConfigService configService) { + try { + configService.getConfig("Dubbo-Nacos-Test", "Dubbo-Nacos-Test", 3000L); + return true; + } catch (NacosException e) { + return false; + } + } private Properties buildNacosProperties(URL url) { Properties properties = new Properties(); diff --git a/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java index 9159e076fc..8ff2d826f3 100644 --- a/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java +++ b/dubbo-metadata/dubbo-metadata-report-nacos/src/test/java/org/apache/dubbo/metadata/store/nacos/RetryTest.java @@ -27,6 +27,7 @@ import org.mockito.Mockito; import com.alibaba.nacos.api.NacosFactory; import com.alibaba.nacos.api.config.ConfigService; +import com.alibaba.nacos.api.exception.NacosException; import static com.alibaba.nacos.client.constant.Constants.HealthCheck.DOWN; import static com.alibaba.nacos.client.constant.Constants.HealthCheck.UP; @@ -82,4 +83,39 @@ class RetryTest { } } } + + @Test + void testRequest() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + ConfigService mock = new MockConfigService() { + @Override + public String getConfig(String dataId, String group, long timeoutMs) throws NacosException { + if (atomicInteger.incrementAndGet() > 10) { + return ""; + } else { + throw new NacosException(); + } + } + + @Override + public String getServerStatus() { + return UP; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createConfigService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> new NacosMetadataReport(url)); + + try { + new NacosMetadataReport(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } } diff --git a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java index b7809911a2..32c61034a6 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java +++ b/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java @@ -131,7 +131,7 @@ public class NacosNamingServiceUtils { try { for (int i = 0; i < retryTimes + 1; i++) { namingService = NacosFactory.createNamingService(nacosProperties); - if (!check || UP.equals(namingService.getServerStatus())) { + if (!check || (UP.equals(namingService.getServerStatus()) && testNamingService(namingService))) { break; } else { logger.warn(LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION, "", "", @@ -160,6 +160,15 @@ public class NacosNamingServiceUtils { return new NacosNamingServiceWrapper(namingService, retryTimes, sleepMsBetweenRetries); } + private static boolean testNamingService(NamingService namingService) { + try { + namingService.getAllInstances("Dubbo-Nacos-Test", false); + return true; + } catch (NacosException e) { + return false; + } + } + private static Properties buildNacosProperties(URL url) { Properties properties = new Properties(); setServerAddr(url, properties); diff --git a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java index 35c13d6a41..b2e0b4e515 100644 --- a/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java +++ b/dubbo-registry/dubbo-registry-nacos/src/test/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtilsTest.java @@ -17,6 +17,7 @@ package org.apache.dubbo.registry.nacos.util; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.concurrent.atomic.AtomicInteger; @@ -96,6 +97,65 @@ class NacosNamingServiceUtilsTest { nacosFactoryMockedStatic.when(() -> NacosFactory.createNamingService((Properties) any())).thenReturn(mock); + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10); + Assertions.assertThrows(IllegalStateException.class, () -> NacosNamingServiceUtils.createNamingService(url)); + + try { + NacosNamingServiceUtils.createNamingService(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } + + @Test + void testDisable() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + NamingService mock = new MockNamingService() { + @Override + public String getServerStatus() { + return DOWN; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createNamingService((Properties) any())).thenReturn(mock); + + + URL url = URL.valueOf("nacos://127.0.0.1:8848") + .addParameter("nacos.retry", 5) + .addParameter("nacos.retry-wait", 10) + .addParameter("nacos.check", "false"); + try { + NacosNamingServiceUtils.createNamingService(url); + } catch (Throwable t) { + Assertions.fail(t); + } + } + } + + @Test + void testRequest() { + try (MockedStatic nacosFactoryMockedStatic = Mockito.mockStatic(NacosFactory.class)) { + AtomicInteger atomicInteger = new AtomicInteger(0); + NamingService mock = new MockNamingService() { + @Override + public List getAllInstances(String serviceName, boolean subscribe) throws NacosException { + if (atomicInteger.incrementAndGet() > 10) { + return null; + } else { + throw new NacosException(); + } + } + + @Override + public String getServerStatus() { + return UP; + } + }; + nacosFactoryMockedStatic.when(() -> NacosFactory.createNamingService((Properties) any())).thenReturn(mock); + + URL url = URL.valueOf("nacos://127.0.0.1:8848") .addParameter("nacos.retry", 5) .addParameter("nacos.retry-wait", 10); From aff1ee6f0d84b89e985d9af0669aa6c6287bf05f Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Mon, 16 Jan 2023 20:59:50 +0800 Subject: [PATCH 07/11] Fix instance revision check (#11314) --- .../dubbo/registry/client/AbstractServiceDiscovery.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java index d3431d77e9..6241fe1778 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java @@ -309,8 +309,9 @@ public abstract class AbstractServiceDiscovery implements ServiceDiscovery { protected void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) { this.doUnregister(oldServiceInstance); - if (!EMPTY_REVISION.equals(getExportedServicesRevision(serviceInstance))) { - this.serviceInstance = newServiceInstance; + this.serviceInstance = newServiceInstance; + + if (!EMPTY_REVISION.equals(getExportedServicesRevision(newServiceInstance))) { reportMetadata(newServiceInstance.getServiceMetadata()); this.doRegister(newServiceInstance); } From 39eedea99f23dc073768679e8f528fb418de6933 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Mon, 16 Jan 2023 21:10:12 +0800 Subject: [PATCH 08/11] Update release test for 3.1 --- .github/workflows/release-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release-test.yml b/.github/workflows/release-test.yml index 1f15b5a62d..5e84dce810 100644 --- a/.github/workflows/release-test.yml +++ b/.github/workflows/release-test.yml @@ -238,7 +238,7 @@ jobs: strategy: fail-fast: false matrix: - jdk: [ 8, 11, 17, 19 ] + jdk: [ 8, 11 ] job_id: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v3 @@ -295,7 +295,7 @@ jobs: JAVA_VER: ${{matrix.jdk}} strategy: matrix: - jdk: [ 8, 11, 17, 19 ] + jdk: [ 8, 11 ] steps: - uses: actions/checkout@v3 with: From 8cc353962f5482f20ac2d31054707b4ca604a127 Mon Sep 17 00:00:00 2001 From: earthchen Date: Tue, 17 Jan 2023 11:13:07 +0800 Subject: [PATCH 09/11] Serialization and deserialization use classes in the wrapper (#11302) --- .../apache/dubbo/common/utils/ClassUtils.java | 9 +-- .../tri/ReflectionPackableMethod.java | 61 +++++++++++++------ 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ClassUtils.java b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ClassUtils.java index 83c9af30a9..a403519318 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ClassUtils.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/utils/ClassUtils.java @@ -113,6 +113,7 @@ public class ClassUtils { PRIMITIVE_WRAPPER_TYPE_MAP.put(Integer.class, int.class); PRIMITIVE_WRAPPER_TYPE_MAP.put(Long.class, long.class); PRIMITIVE_WRAPPER_TYPE_MAP.put(Short.class, short.class); + PRIMITIVE_WRAPPER_TYPE_MAP.put(Void.class, void.class); Set> primitiveTypeNames = new HashSet<>(32); primitiveTypeNames.addAll(PRIMITIVE_WRAPPER_TYPE_MAP.values()); @@ -163,7 +164,7 @@ public class ClassUtils { if (cl == null) { try { cl = Thread.currentThread().getContextClassLoader(); - } catch (Throwable ex) { + } catch (Exception ignored) { // Cannot access thread context ClassLoader - falling back to system class loader... } if (cl == null) { @@ -173,7 +174,7 @@ public class ClassUtils { // getClassLoader() returning null indicates the bootstrap ClassLoader try { cl = ClassLoader.getSystemClassLoader(); - } catch (Throwable ex) { + } catch (Exception ignored) { // Cannot access system ClassLoader - oh well, maybe the caller can live with null... } } @@ -475,7 +476,7 @@ public class ClassUtils { public static boolean isPresent(String className, ClassLoader classLoader) { try { forName(className, classLoader); - } catch (Throwable ignored) { // Ignored + } catch (Exception ignored) { // Ignored return false; } return true; @@ -493,7 +494,7 @@ public class ClassUtils { Class targetClass = null; try { targetClass = forName(className, classLoader); - } catch (Throwable ignored) { // Ignored + } catch (Exception ignored) { // Ignored } return targetClass; } diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java index 48015d55af..c952a1ca27 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java @@ -22,6 +22,7 @@ import org.apache.dubbo.common.constants.CommonConstants; import org.apache.dubbo.common.serialize.MultipleSerialization; import org.apache.dubbo.common.serialize.support.DefaultSerializationSelector; import org.apache.dubbo.common.stream.StreamObserver; +import org.apache.dubbo.common.utils.ClassUtils; import org.apache.dubbo.config.Constants; import org.apache.dubbo.rpc.model.MethodDescriptor; import org.apache.dubbo.rpc.model.PackableMethod; @@ -33,7 +34,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.util.Iterator; -import java.util.stream.Stream; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import static org.apache.dubbo.common.constants.CommonConstants.$ECHO; import static org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME; @@ -91,8 +93,7 @@ public class ReflectionPackableMethod implements PackableMethod { .getExtension(url.getParameter(Constants.MULTI_SERIALIZATION_KEY, CommonConstants.DEFAULT_KEY)); - this.requestPack = new WrapRequestPack(serialization, url, serializeName, actualRequestTypes, - singleArgument); + this.requestPack = new WrapRequestPack(serialization, url, serializeName, singleArgument); this.responsePack = new WrapResponsePack(serialization, url, actualResponseType); this.requestUnpack = new WrapRequestUnpack(serialization, url, actualRequestTypes); this.responseUnpack = new WrapResponseUnpack(serialization, url, actualResponseType); @@ -322,10 +323,16 @@ public class ReflectionPackableMethod implements PackableMethod { @Override public byte[] pack(Object obj) throws IOException { ByteArrayOutputStream bos = new ByteArrayOutputStream(); - multipleSerialization.serialize(url, serialize, actualResponseType, obj, bos); + Class clz; + if (obj != null) { + clz = obj.getClass(); + } else { + clz = actualResponseType; + } + multipleSerialization.serialize(url, serialize, clz, obj, bos); return TripleCustomerProtocolWapper.TripleResponseWrapper.Builder.newBuilder() .setSerializeType(serialize) - .setType(actualResponseType.getName()) + .setType(clz.getName()) .setData(bos.toByteArray()) .build() .toByteArray(); @@ -334,15 +341,17 @@ public class ReflectionPackableMethod implements PackableMethod { private static class WrapResponseUnpack implements UnPack { + private final Map> classCache = new ConcurrentHashMap<>(); + private final MultipleSerialization serialization; private final URL url; - private final Class returnClass; + private final Class actualResponseType; - private WrapResponseUnpack(MultipleSerialization serialization, URL url, Class returnClass) { + private WrapResponseUnpack(MultipleSerialization serialization, URL url, Class actualResponseType) { this.serialization = serialization; this.url = url; - this.returnClass = returnClass; + this.actualResponseType = actualResponseType; } @Override @@ -351,7 +360,8 @@ public class ReflectionPackableMethod implements PackableMethod { .parseFrom(data); final String serializeType = convertHessianFromWrapper(wrapper.getSerializeType()); ByteArrayInputStream bais = new ByteArrayInputStream(wrapper.getData()); - return serialization.deserialize(url, serializeType, returnClass, bais); + Class clz = getClassFromCache(wrapper.getType(), classCache, actualResponseType); + return serialization.deserialize(url, serializeType, clz, bais); } } @@ -359,21 +369,16 @@ public class ReflectionPackableMethod implements PackableMethod { private final String serialize; private final MultipleSerialization multipleSerialization; - private final String[] argumentsType; private final URL url; private final boolean singleArgument; - private final Class[] actualRequestTypes; private WrapRequestPack(MultipleSerialization multipleSerialization, URL url, String serialize, - Class[] actualRequestTypes, boolean singleArgument) { this.url = url; this.serialize = convertHessianToWrapper(serialize); this.multipleSerialization = multipleSerialization; - this.actualRequestTypes = actualRequestTypes; - this.argumentsType = Stream.of(actualRequestTypes).map(Class::getName).toArray(String[]::new); this.singleArgument = singleArgument; } @@ -387,10 +392,8 @@ public class ReflectionPackableMethod implements PackableMethod { } final TripleCustomerProtocolWapper.TripleRequestWrapper.Builder builder = TripleCustomerProtocolWapper.TripleRequestWrapper.Builder.newBuilder(); builder.setSerializeType(serialize); - for (String type : argumentsType) { - builder.addArgTypes(type); - } for (Object argument : arguments) { + builder.addArgTypes(argument.getClass().getName()); ByteArrayOutputStream bos = new ByteArrayOutputStream(); multipleSerialization.serialize(url, serialize, argument.getClass(), argument, bos); builder.addArgs(bos.toByteArray()); @@ -433,6 +436,8 @@ public class ReflectionPackableMethod implements PackableMethod { private class WrapRequestUnpack implements UnPack { + private final Map> classCache = new ConcurrentHashMap<>(); + private final MultipleSerialization serialization; private final URL url; @@ -453,11 +458,27 @@ public class ReflectionPackableMethod implements PackableMethod { for (int i = 0; i < wrapper.getArgs().size(); i++) { ByteArrayInputStream bais = new ByteArrayInputStream( wrapper.getArgs().get(i)); - ret[i] = serialization.deserialize(url, wrapper.getSerializeType(), - actualRequestTypes[i], - bais); + String className = wrapper.getArgTypes().get(i); + Class clz = getClassFromCache(className, classCache, actualRequestTypes[i]); + ret[i] = serialization.deserialize(url, wrapper.getSerializeType(), clz, bais); } return ret; } + + + } + + + private static Class getClassFromCache(String className, Map> classCache, Class expectedClass) { + Class clz = classCache.get(className); + if (clz == null) { + try { + clz = ClassUtils.forName(className); + } catch (ClassNotFoundException e) { + clz = expectedClass; + } + classCache.put(className, clz); + } + return clz; } } From d5e9c58846b44476b25338dba742d9cc13f111c7 Mon Sep 17 00:00:00 2001 From: earthchen Date: Tue, 17 Jan 2023 11:17:27 +0800 Subject: [PATCH 10/11] change class loader (#11312) --- .../call/ReflectionAbstractServerCall.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/ReflectionAbstractServerCall.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/ReflectionAbstractServerCall.java index 200ccf2412..214edbe3ea 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/ReflectionAbstractServerCall.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/call/ReflectionAbstractServerCall.java @@ -46,13 +46,13 @@ public class ReflectionAbstractServerCall extends AbstractServerCall { private List methodDescriptors; public ReflectionAbstractServerCall(Invoker invoker, - ServerStream serverStream, - FrameworkModel frameworkModel, - String acceptEncoding, - String serviceName, - String methodName, - List headerFilters, - Executor executor) { + ServerStream serverStream, + FrameworkModel frameworkModel, + String acceptEncoding, + String serviceName, + String methodName, + List headerFilters, + Executor executor) { super(invoker, serverStream, frameworkModel, getServiceDescriptor(invoker.getUrl()), acceptEncoding, serviceName, methodName, @@ -155,10 +155,8 @@ public class ReflectionAbstractServerCall extends AbstractServerCall { if (isClosed()) { return null; } - if (serviceDescriptor != null) { - ClassLoadUtil.switchContextLoader( - serviceDescriptor.getServiceInterfaceClass().getClassLoader()); - } + ClassLoadUtil.switchContextLoader( + invoker.getUrl().getServiceModel().getClassLoader()); return packableMethod.getRequestUnpack().unpack(data); } From b6c5a26606126e701d2070490beabf84112d0d26 Mon Sep 17 00:00:00 2001 From: Albumen Kevin Date: Tue, 17 Jan 2023 11:17:34 +0800 Subject: [PATCH 11/11] Enhance class forname in triple (#11316) --- .../tri/ReflectionPackableMethod.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java index c952a1ca27..cab311698c 100644 --- a/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java +++ b/dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/ReflectionPackableMethod.java @@ -17,6 +17,14 @@ package org.apache.dubbo.rpc.protocol.tri; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.lang.reflect.ParameterizedType; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + import org.apache.dubbo.common.URL; import org.apache.dubbo.common.constants.CommonConstants; import org.apache.dubbo.common.serialize.MultipleSerialization; @@ -29,14 +37,6 @@ import org.apache.dubbo.rpc.model.PackableMethod; import com.google.protobuf.Message; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.lang.reflect.ParameterizedType; -import java.util.Iterator; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - import static org.apache.dubbo.common.constants.CommonConstants.$ECHO; import static org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME; import static org.apache.dubbo.remoting.Constants.SERIALIZATION_KEY; @@ -470,11 +470,16 @@ public class ReflectionPackableMethod implements PackableMethod { private static Class getClassFromCache(String className, Map> classCache, Class expectedClass) { + if (expectedClass.getName().equals(className)) { + return expectedClass; + } + Class clz = classCache.get(className); if (clz == null) { try { clz = ClassUtils.forName(className); - } catch (ClassNotFoundException e) { + } catch (Throwable e) { + // To catch IllegalStateException, LinkageError, ClassNotFoundException clz = expectedClass; } classCache.put(className, clz);