Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.dubbo.common.extension.Adaptive;
import org.apache.dubbo.common.extension.SPI;

import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.PROTOCOL_KEY;
import static org.apache.dubbo.common.extension.ExtensionScope.APPLICATION;

Expand Down Expand Up @@ -47,4 +48,8 @@ public interface RegistryFactory {
*/
@Adaptive({PROTOCOL_KEY})
Registry getRegistry(URL url);

static boolean isCheck(URL url) {
return url.getParameter(CHECK_KEY, true) && url.getPort() != 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.apache.dubbo.rpc.model.ApplicationModel;
import org.apache.dubbo.rpc.model.ScopeModelAware;

import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.INTERFACE_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.TIMESTAMP_KEY;
import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_FAILED_CREATE_INSTANCE;
Expand Down Expand Up @@ -74,7 +73,7 @@ public Registry getRegistry(URL url) {

String key = createRegistryCacheKey(url);
Registry registry = null;
boolean check = url.getParameter(CHECK_KEY, true) && url.getPort() != 0;
boolean check = RegistryFactory.isCheck(url);

// Lock the registry access process to ensure a single instance of the registry
registryManager.getRegistryLock().lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ public class NacosConnectionManager {

private final boolean check;

private final Properties nacosProperties;

public NacosConnectionManager(URL connectionURL, boolean check, int retryTimes, int sleepMsBetweenRetries) {
this.connectionURL = connectionURL;
this.check = check;
this.retryTimes = retryTimes;
this.sleepMsBetweenRetries = sleepMsBetweenRetries;
this.nacosProperties = buildNacosProperties(this.connectionURL);
// create default one
this.namingServiceList.add(createNamingService());
}
Expand All @@ -78,6 +81,7 @@ protected NacosConnectionManager(NamingService namingService) {
this.retryTimes = 0;
this.sleepMsBetweenRetries = 0;
this.check = false;
this.nacosProperties = null;
// create default one
this.namingServiceList.add(namingService);
}
Expand Down Expand Up @@ -116,7 +120,7 @@ public synchronized void shutdownAll() {
* @return {@link NamingService}
*/
protected NamingService createNamingService() {
Properties nacosProperties = buildNacosProperties(this.connectionURL);
// Properties nacosProperties = buildNacosProperties(this.connectionURL);
NamingService namingService = null;
try {
for (int i = 0; i < retryTimes + 1; i++) {
Expand Down Expand Up @@ -177,6 +181,9 @@ private boolean testNamingService(NamingService namingService) {

private Properties buildNacosProperties(URL url) {
Properties properties = new Properties();
if (StringUtils.isEmpty(url.getHost())) {
return properties;
}
setServerAddr(url, properties);
setProperties(url, properties);
return properties;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.registry.RegistryFactory;
import org.apache.dubbo.registry.client.DefaultServiceInstance;
import org.apache.dubbo.registry.client.ServiceInstance;
import org.apache.dubbo.registry.nacos.NacosConnectionManager;
Expand Down Expand Up @@ -114,8 +115,17 @@ public static NacosNamingServiceWrapper createNamingService(URL connectionURL) {
boolean check = connectionURL.getParameter(NACOS_CHECK_KEY, true);
int retryTimes = connectionURL.getPositiveParameter(NACOS_RETRY_KEY, 10);
int sleepMsBetweenRetries = connectionURL.getPositiveParameter(NACOS_RETRY_WAIT_KEY, 10);
NacosConnectionManager nacosConnectionManager =
new NacosConnectionManager(connectionURL, check, retryTimes, sleepMsBetweenRetries);
NacosConnectionManager nacosConnectionManager;
try {
nacosConnectionManager =
new NacosConnectionManager(connectionURL, check, retryTimes, sleepMsBetweenRetries);
} catch (IllegalStateException e) {
if (RegistryFactory.isCheck(connectionURL)) {
throw e;
}
nacosConnectionManager =
new NacosConnectionManager(connectionURL, false, retryTimes, sleepMsBetweenRetries);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change might cause some problems. This registry isn’t functioning normally, but it's still added to the RegistryManager. As a result, subsequent actions might use this registry, which could lead to more errors. I'm not sure exactly what will happen after this PR is merged, but I’ll try to reproduce the issue later.

Copy link
Contributor

@RainYuY RainYuY Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After trying it out, I got the result.

23:41:19.981 |-WARN  [DubboRegistryRetryTimer-thread-1] registry.nacos.NacosNamingServiceWrapper:    -|  [DUBBO] Failed to request nacos naming server. Dubbo will try to retry in 10. Try times: 6, dubbo version: 3.3.6-SNAPSHOT, current host: 192.168.31.224, error code: 1-37. This may be caused by , go to https://dubbo.apache.org/faq/1/37 to find instructions. 
com.alibaba.nacos.api.exception.NacosException: Client not connected, current status:STARTING
	at com.alibaba.nacos.common.remote.client.RpcClient.request(RpcClient.java:645) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.common.remote.client.RpcClient.request(RpcClient.java:624) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.remote.gprc.NamingGrpcClientProxy.requestToServer(NamingGrpcClientProxy.java:449) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.remote.gprc.NamingGrpcClientProxy.doRegisterService(NamingGrpcClientProxy.java:252) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.remote.gprc.NamingGrpcClientProxy.registerServiceForEphemeral(NamingGrpcClientProxy.java:147) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.remote.gprc.NamingGrpcClientProxy.registerService(NamingGrpcClientProxy.java:138) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.remote.NamingClientProxyDelegate.registerService(NamingClientProxyDelegate.java:96) ~[nacos-client-2.5.1.jar:?]
	at com.alibaba.nacos.client.naming.NacosNamingService.registerInstance(NacosNamingService.java:161) ~[nacos-client-2.5.1.jar:?]
	at org.apache.dubbo.registry.nacos.NacosNamingServiceWrapper.lambda$registerInstance$5(NacosNamingServiceWrapper.java:133) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.registry.nacos.NacosNamingServiceWrapper.accept(NacosNamingServiceWrapper.java:495) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.registry.nacos.NacosNamingServiceWrapper.registerInstance(NacosNamingServiceWrapper.java:133) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.registry.nacos.NacosRegistry.doRegister(NacosRegistry.java:205) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.registry.retry.FailedRegisteredTask.doRetry(FailedRegisteredTask.java:33) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.registry.retry.AbstractRetryTask.run(AbstractRetryTask.java:127) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.common.timer.HashedWheelTimer$HashedWheelTimeout.expire(HashedWheelTimer.java:654) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.common.timer.HashedWheelTimer$HashedWheelBucket.expireTimeouts(HashedWheelTimer.java:733) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at org.apache.dubbo.common.timer.HashedWheelTimer$Worker.run(HashedWheelTimer.java:455) ~[dubbo-3.3.6-SNAPSHOT.jar:3.3.6-SNAPSHOT]
	at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]

It will return to normal once the Nacos server is back online.
It's different from metadata or the config center — I think they should follow the same behavior.
This needs some discussion.

@zrlw @AlbumenJ @oxsean @heliang666s

}
return new NacosNamingServiceWrapper(nacosConnectionManager, retryTimes, sleepMsBetweenRetries);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.CollectionUtils;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.registry.RegistryFactory;
import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
import org.apache.dubbo.registry.client.ServiceDiscovery;
import org.apache.dubbo.registry.client.ServiceInstance;
import org.apache.dubbo.registry.client.event.ServiceInstancesChangedEvent;
import org.apache.dubbo.registry.client.event.listener.ServiceInstancesChangedListener;
import org.apache.dubbo.remoting.zookeeper.curator5.ZookeeperClient;
import org.apache.dubbo.rpc.RpcException;
import org.apache.dubbo.rpc.model.ApplicationModel;

Expand Down Expand Up @@ -80,8 +82,19 @@ public class ZookeeperServiceDiscovery extends AbstractServiceDiscovery {

public ZookeeperServiceDiscovery(ApplicationModel applicationModel, URL registryURL) {
super(applicationModel, registryURL);
boolean check = RegistryFactory.isCheck(registryURL);
CuratorFramework curatorFramework;
try {
this.curatorFramework = buildCuratorFramework(registryURL, this);
try {
curatorFramework = buildCuratorFramework(registryURL, this);
} catch (IllegalStateException e) {
if (check) {
throw e;
}
curatorFramework = buildCuratorFramework(
registryURL.addParameter(ZookeeperClient.ZOOKEEPER_CHECK_KEY, false), this);
}
this.curatorFramework = curatorFramework;
this.rootPath = getRootPath(registryURL);
this.serviceDiscovery = buildServiceDiscovery(curatorFramework, rootPath);
this.serviceDiscovery.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.dubbo.registry.client.ServiceInstance;
import org.apache.dubbo.registry.zookeeper.ZookeeperInstance;
import org.apache.dubbo.registry.zookeeper.ZookeeperServiceDiscovery;
import org.apache.dubbo.remoting.zookeeper.curator5.ZookeeperClient;
import org.apache.dubbo.rpc.model.ScopeModelUtil;

import java.lang.reflect.Method;
Expand Down Expand Up @@ -117,7 +118,8 @@ public List<ACL> getAclForPath(String path) {
throw new IllegalStateException("zookeeper client initialization failed");
}

if (!curatorFramework.getZookeeperClient().isConnected()) {
boolean check = connectionURL.getParameter(ZookeeperClient.ZOOKEEPER_CHECK_KEY, true);
if (check && !curatorFramework.getZookeeperClient().isConnected()) {
throw new IllegalStateException("failed to connect to zookeeper server");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.apache.curator.x.discovery.ServiceDiscoveryBuilder;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -50,6 +51,7 @@
import org.mockito.internal.util.collections.Sets;

import static java.util.Arrays.asList;
import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -84,23 +86,20 @@ class ZookeeperServiceDiscoveryTest {
private ServiceDiscovery mockServiceDiscovery;
private ServiceCacheBuilder mockServiceCacheBuilder;
private ServiceCache mockServiceCache;
private static final CuratorFrameworkFactory.Builder spyBuilder = spy(CuratorFrameworkFactory.builder());

@BeforeAll
public static void beforeAll() {
zookeeperConnectionAddress1 = "zookeeper://localhost:" + "2181";
}

@BeforeEach
public void init() throws Exception {
// mock begin
// create mock bean begin
CuratorFrameworkFactory.Builder realBuilder = CuratorFrameworkFactory.builder();
CuratorFrameworkFactory.Builder spyBuilder = spy(realBuilder);
curatorFrameworkFactoryMockedStatic = mockStatic(CuratorFrameworkFactory.class);
curatorFrameworkFactoryMockedStatic
.when(CuratorFrameworkFactory::builder)
.thenReturn(spyBuilder);
serviceDiscoveryBuilderMockedStatic = mockStatic(ServiceDiscoveryBuilder.class);
}

@BeforeEach
public void init() throws Exception {
mockServiceDiscoveryBuilder = mock(ServiceDiscoveryBuilder.class);
mockServiceDiscovery = mock(ServiceDiscovery.class);
mockServiceCacheBuilder = mock(ServiceCacheBuilder.class);
Expand Down Expand Up @@ -200,6 +199,32 @@ public void onEvent(ServiceInstancesChangedEvent event) {
assertTrue(serviceInstances.isEmpty());
}

@Test
void testRegistryCheckConnectDefault() {
when(mockCuratorZookeeperClient.isConnected()).thenReturn(false);

URL registryUrl = URL.valueOf(zookeeperConnectionAddress1);
ApplicationModel applicationModel = ApplicationModel.defaultModel();
registryUrl.setScopeModel(applicationModel);

Assertions.assertThrowsExactly(IllegalStateException.class, () -> {
new ZookeeperServiceDiscovery(applicationModel, registryUrl);
});
}

@Test
void testRegistryNotCheckConnect() {
when(mockCuratorZookeeperClient.isConnected()).thenReturn(false);

URL registryUrl = URL.valueOf(zookeeperConnectionAddress1).addParameter(CHECK_KEY, false);
ApplicationModel applicationModel = ApplicationModel.defaultModel();
registryUrl.setScopeModel(applicationModel);

Assertions.assertDoesNotThrow(() -> {
new ZookeeperServiceDiscovery(applicationModel, registryUrl);
});
}

@AfterAll
public static void afterAll() throws Exception {
if (curatorFrameworkFactoryMockedStatic != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.dubbo.registry.client.DefaultServiceInstance;
import org.apache.dubbo.registry.client.ServiceInstance;
import org.apache.dubbo.registry.zookeeper.ZookeeperInstance;
import org.apache.dubbo.remoting.zookeeper.curator5.ZookeeperClient;
import org.apache.dubbo.rpc.model.ApplicationModel;

import java.util.Arrays;
Expand Down Expand Up @@ -109,6 +110,25 @@ void testBuildServiceDiscovery() throws Exception {
curatorFramework.getZookeeperClient().close();
}

@Test
void testBuildCuratorFrameworkCheckConnectDefault() {
when(mockCuratorZookeeperClient.isConnected()).thenReturn(false);
Assertions.assertThrowsExactly(IllegalStateException.class, () -> {
CuratorFramework curatorFramework = CuratorFrameworkUtils.buildCuratorFramework(registryUrl, null);
curatorFramework.getZookeeperClient().close();
});
}

@Test
void testBuildCuratorFrameworkNotCheckConnect() {
when(mockCuratorZookeeperClient.isConnected()).thenReturn(false);
URL url = registryUrl.addParameter(ZookeeperClient.ZOOKEEPER_CHECK_KEY, false);
Assertions.assertDoesNotThrow(() -> {
CuratorFramework curatorFramework = CuratorFrameworkUtils.buildCuratorFramework(url, null);
curatorFramework.getZookeeperClient().close();
});
}

@Test
void testBuild() {
ServiceInstance dubboServiceInstance =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ public List<ACL> getAclForPath(String path) {
client.getConnectionStateListenable().addListener(new CuratorConnectionStateListener(url));
client.start();
boolean connected = client.blockUntilConnected(timeout, TimeUnit.MILLISECONDS);

if (!connected) {
boolean check = url.getParameter(ZOOKEEPER_CHECK_KEY, true);
if (check && !connected) {
IllegalStateException illegalStateException =
new IllegalStateException("zookeeper not connected, the address is: " + url);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* Common abstraction of Zookeeper client.
*/
public interface ZookeeperClient {
String ZOOKEEPER_CHECK_KEY = "zookeeper.check";

/**
* Create ZNode in Zookeeper.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.apache.dubbo.common.constants.CommonConstants.TIMEOUT_KEY;
import static org.apache.dubbo.common.constants.LoggerCodeConstants.TRANSPORT_FAILED_DESTROY_ZOOKEEPER;

Expand Down Expand Up @@ -85,8 +86,17 @@ public ZookeeperClient connect(URL url) {
logger.info("find valid zookeeper client from the cache for address: " + url);
return zookeeperClient;
}
boolean check = url.getParameter(CHECK_KEY, true) && url.getPort() != 0;

zookeeperClient = new Curator5ZookeeperClient(url);
try {
zookeeperClient = new Curator5ZookeeperClient(url);
} catch (IllegalStateException e) {
if (check) {
throw e;
}
zookeeperClient =
new Curator5ZookeeperClient(url.addParameter(ZookeeperClient.ZOOKEEPER_CHECK_KEY, false));
}
logger.info("No valid zookeeper client found from cache, therefore create a new client for url. " + url);
writeToClientMap(addressList, zookeeperClient);
}
Expand Down
Loading
Loading