You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "fantiq (via GitHub)" <gi...@apache.org> on 2025/10/01 00:13:42 UTC
[PR] feat(registry): support the option, whether to check the connection … [dubbo]
fantiq opened a new pull request, #15594:
URL: https://github.com/apache/dubbo/pull/15594
## What is the purpose of the change?
resolve #15271 #15003 , this issues are related to the config option `registry.check`.
When `registry.check=true` (default value is `true`) is set, the connection status will be checked when creating the registry client. if the check fails, an exception will be thrown.
when `registry.check=false` is set, no check is performed and no exception is ever thrown.
The reason for NEP is caused by setting the option `registry.check` to `false` and throwning an exception.
so here when `registry.check=false` no exception should be thrown. that is, the connection status should not be checked.
Currently, in the nacos client, the option `nacos.check` is used to determine whether to check the connection status or not.
the zookeeper client does not yet support not checking the connection status.
This PR add an new option `zookeeper.check`, it is similar to option `nacos.check`.
When creating a registry client, it will first be created using the default behavior. if an exception is thrown and `registry.check=false`,
it will try to create the registry client again by turning on the option (`nacos.check=false` or `zookeeper.check=false`) to not check the connection status.
## Checklist
- [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test to verify your logic correction. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project.
- [x] Make sure gitHub actions can pass. [Why the workflow is failing and how to fix it?](../CONTRIBUTING.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2242280383
##########
dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java:
##########
@@ -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);
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2246924602
##########
dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java:
##########
@@ -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);
Review Comment:
> I don't think we need a special key for ZK. How about using CHECK_KEY?
agree with u
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3142933935
Thank you very mush for your preview.
In the zookeeper registry client, i have modified it to use `registry.check` directly instead of introducing new specific options.
Can it be considered that `nacos.check` actually replaces the function of `registry.check`?
So here i only need to be compatible with this situation which led to #15003.
When `registry.check = false` and `nacos.check = true`, the value of `nacos.check` should be `false` actually.
And then i will evaluate the impact of this change on the config center and metadata center.
@RainYuY @zrlw need more review, thx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2242258878
##########
dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java:
##########
@@ -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);
Review Comment:
I think this logic can be moved to the abstract class, since both ZK and Nacos share the same logic.
##########
dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java:
##########
@@ -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);
Review Comment:
I don't think we need a special key for ZK. How about using CHECK_KEY?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2250876518
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/AbstractRetryTask.java:
##########
@@ -124,6 +124,9 @@ public void run(Timeout timeout) throws Exception {
logger.info(taskName + " : " + url);
}
try {
+ if (!registry.isAvailable()) {
+ throw new IllegalStateException("Registry is not available.");
Review Comment:
Please use log.error not exception. Because the Exception will be catched.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3143726710
> Thank you very mush for your preview.
>
> In the zookeeper registry client, i have modified it to use `registry.check` directly instead of introducing new specific options.
>
> Can it be considered that `nacos.check` actually replaces the function of `registry.check`? So here i only need to be compatible with this situation which led to #15003. When `registry.check = false` and `nacos.check = true`, the value of `nacos.check` should be `false` actually.
>
> And then i will evaluate the impact of this change on the config center and metadata center.
>
> @RainYuY @zrlw need more review, thx
The modification is OK, and I even think the nacos.check configuration item could be removed in the future.
Also, what I mentioned earlier was not that this PR affects the config center or metadata, but that their behavior is inconsistent. In the current implementation of this PR, even if the check fails, the registry is still initialized. As a result, the service keeps trying to register to an unavailable registry.
I think a better approach would be not to repeatedly attempt registration when the registry is unavailable. Instead, we should first check the availability of the registry, and only proceed with registration once it becomes available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3134770773
@RainYuY @zrlw PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3148864165
> Also, what I mentioned earlier was not that this PR affects the config center or metadata, but that their behavior is inconsistent
@RainYuY Your focus is right. I have checked the startup process of the config-center and metadata. they both support the `check` option and need to be modified to support the logic of not checking the connection status.
I will continue to modify these later.
> the service keeps trying to register to an unavailable registry
About this situation, is it possible to add connection status check logic before register?And the retry task should be retained.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2243143960
##########
dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java:
##########
@@ -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);
Review Comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3131033901
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15594?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `36.36364%` with `21 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 31.66%. Comparing base ([`55acb38`](https://app.codecov.io/gh/apache/dubbo/commit/55acb38670a6777cb6778c72b7c39d373bdf877d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7f027a8`](https://app.codecov.io/gh/apache/dubbo/commit/7f027a8843c488a7a4ad04422a7f55c29acad897?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: **Current head 7f027a8 differs from pull request most recent head 2642ddb**
Please [upload](https://docs.codecov.com/docs/codecov-uploader) reports for the commit 2642ddb to get more accurate results.
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15594?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ing/zookeeper/curator5/ZookeeperClientManager.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-zookeeper-curator5%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Fzookeeper%2Fcurator5%2FZookeeperClientManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyLWN1cmF0b3I1L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvY3VyYXRvcjUvWm9va2VlcGVyQ2xpZW50TWFuYWdlci5qYXZh) | 25.00% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../registry/zookeeper/ZookeeperServiceDiscovery.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-zookeeper%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fzookeeper%2FZookeeperServiceDiscovery.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS96b29rZWVwZXIvWm9va2VlcGVyU2VydmljZURpc2NvdmVyeS5qYXZh) | 44.44% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...o/registry/nacos/util/NacosNamingServiceUtils.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-nacos%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fnacos%2Futil%2FNacosNamingServiceUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktbmFjb3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlZ2lzdHJ5L25hY29zL3V0aWwvTmFjb3NOYW1pbmdTZXJ2aWNlVXRpbHMuamF2YQ==) | 33.33% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/dubbo/registry/nacos/NacosConnectionManager.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-nacos%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fnacos%2FNacosConnectionManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktbmFjb3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlZ2lzdHJ5L25hY29zL05hY29zQ29ubmVjdGlvbk1hbmFnZXIuamF2YQ==) | 25.00% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/dubbo/registry/RegistryFactory.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2FRegistryFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9SZWdpc3RyeUZhY3RvcnkuamF2YQ==) | 0.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...registry/zookeeper/util/CuratorFrameworkUtils.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-zookeeper%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fzookeeper%2Futil%2FCuratorFrameworkUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktem9va2VlcGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS96b29rZWVwZXIvdXRpbC9DdXJhdG9yRnJhbWV3b3JrVXRpbHMuamF2YQ==) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ng/zookeeper/curator5/Curator5ZookeeperClient.java](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-zookeeper-curator5%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Fzookeeper%2Fcurator5%2FCurator5ZookeeperClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3Rpbmctem9va2VlcGVyLWN1cmF0b3I1L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy96b29rZWVwZXIvY3VyYXRvcjUvQ3VyYXRvcjVab29rZWVwZXJDbGllbnQuamF2YQ==) | 50.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15594?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (55acb38) and HEAD (7f027a8). Click for more details.
>
> <details><summary>HEAD has 2 uploads less than BASE</summary>
>
>| Flag | BASE (55acb38) | HEAD (7f027a8) |
>|------|------|------|
>|integration-tests-java21|1|0|
>|integration-tests-java8|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.3 #15594 +/- ##
============================================
- Coverage 37.26% 31.66% -5.61%
+ Complexity 11498 9753 -1745
============================================
Files 1909 1910 +1
Lines 86766 86792 +26
Branches 13010 13014 +4
============================================
- Hits 32337 27480 -4857
- Misses 49914 55243 +5329
+ Partials 4515 4069 -446
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/15594/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [samples-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `31.61% <36.36%> (-0.09%)` | :arrow_down: |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15594/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.21% <36.36%> (-0.14%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo/pull/15594?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3148907270
> @RainYuY Your focus is right. I have checked the startup process of the config-center and metadata. they both support the `check` option and need to be modified to support the logic of not checking the connection status. I will continue to modify these later.
I think there will be a lot of work to do. The best practice might be to open a new PR to reproduce it.
> > the service keeps trying to register to an unavailable registry
>
> About this situation, is it possible to add connection status check logic before register?And the retry task should be retained.
Yes, I think this is the correct action.And it should be solved at this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2439500304
##########
dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/test/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClientManagerTest.java:
##########
@@ -18,36 +18,55 @@
import org.apache.dubbo.common.URL;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.listen.StandardListenerManager;
import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.core.IsNull.nullValue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.mockConstructionWithAnswer;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
class Curator5ZookeeperClientManagerTest {
private ZookeeperClient zookeeperClient;
- private static MockedConstruction<Curator5ZookeeperClient> mockedCurator5ZookeeperClientConstruction;
- private static String zookeeperConnectionAddress1;
+ private static URL zookeeperUrl;
+ private static MockedStatic<CuratorFrameworkFactory> curatorFrameworkFactoryMockedStatic;
+ private CuratorFramework mockCuratorFramework;
@BeforeAll
public static void beforeAll() {
- zookeeperConnectionAddress1 = "zookeeper://127.0.0.1:2181";
- Curator5ZookeeperClient mockCurator5ZookeeperClient = mock(Curator5ZookeeperClient.class);
- mockedCurator5ZookeeperClientConstruction =
- mockConstructionWithAnswer(Curator5ZookeeperClient.class, invocationOnMock -> invocationOnMock
- .getMethod()
- .invoke(mockCurator5ZookeeperClient, invocationOnMock.getArguments()));
+ String zookeeperConnectionAddress1 = "zookeeper://127.0.0.1:2181";
+ zookeeperUrl = URL.valueOf(zookeeperConnectionAddress1 + "/service");
+
+ CuratorFrameworkFactory.Builder realBuilder = CuratorFrameworkFactory.builder();
+ CuratorFrameworkFactory.Builder spyBuilder = spy(realBuilder);
Review Comment:
it might conflict with line 78: CuratorFrameworkFactory.Builder spyBuilder = CuratorFrameworkFactory.builder();
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3413950352
@AlbumenJ @oxsean @zrlw PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2246924143
##########
dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosConnectionManager.java:
##########
@@ -116,7 +120,7 @@ public synchronized void shutdownAll() {
* @return {@link NamingService}
*/
protected NamingService createNamingService() {
- Properties nacosProperties = buildNacosProperties(this.connectionURL);
+ // Properties nacosProperties = buildNacosProperties(this.connectionURL);
Review Comment:
> why not delete it directly?
i missed it there, i'll remove it later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2250876518
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/AbstractRetryTask.java:
##########
@@ -124,6 +124,9 @@ public void run(Timeout timeout) throws Exception {
logger.info(taskName + " : " + url);
}
try {
+ if (!registry.isAvailable()) {
+ throw new IllegalStateException("Registry is not available.");
Review Comment:
Please use log.error not exception. Because the Exception will be catched.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3149747103
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "fantiq (via GitHub)" <gi...@apache.org>.
fantiq commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3149695585
> I think there will be a lot of work to do. The best practice might be to open a new PR to reproduce it.
@RainYuY It's a wise decision to create a new issue. In addition to making the ConfigCenter and MetadataCenter support `check=false`, we also need to consider the processing logic when the connection is recovery, this reqires more discussion.
I will create a new issue later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2243143960
##########
dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/util/NacosNamingServiceUtils.java:
##########
@@ -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);
Review Comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2242128861
##########
dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosConnectionManager.java:
##########
@@ -116,7 +120,7 @@ public synchronized void shutdownAll() {
* @return {@link NamingService}
*/
protected NamingService createNamingService() {
- Properties nacosProperties = buildNacosProperties(this.connectionURL);
+ // Properties nacosProperties = buildNacosProperties(this.connectionURL);
Review Comment:
why not delete it directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY merged PR #15594:
URL: https://github.com/apache/dubbo/pull/15594
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw closed pull request #15594: feat(registry): support the option, whether to check the connection …
URL: https://github.com/apache/dubbo/pull/15594
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3305631792
duplicated with #15639
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#issuecomment-3354196993
@fantiq cc conflicts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat(registry): support the option, whether to check the connection … [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15594:
URL: https://github.com/apache/dubbo/pull/15594#discussion_r2439500304
##########
dubbo-remoting/dubbo-remoting-zookeeper-curator5/src/test/java/org/apache/dubbo/remoting/zookeeper/curator5/Curator5ZookeeperClientManagerTest.java:
##########
@@ -18,36 +18,55 @@
import org.apache.dubbo.common.URL;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.listen.StandardListenerManager;
import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.mockito.MockedConstruction;
+import org.mockito.MockedStatic;
+import static org.apache.dubbo.common.constants.CommonConstants.CHECK_KEY;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsNot.not;
import static org.hamcrest.core.IsNull.nullValue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.mockConstructionWithAnswer;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
class Curator5ZookeeperClientManagerTest {
private ZookeeperClient zookeeperClient;
- private static MockedConstruction<Curator5ZookeeperClient> mockedCurator5ZookeeperClientConstruction;
- private static String zookeeperConnectionAddress1;
+ private static URL zookeeperUrl;
+ private static MockedStatic<CuratorFrameworkFactory> curatorFrameworkFactoryMockedStatic;
+ private CuratorFramework mockCuratorFramework;
@BeforeAll
public static void beforeAll() {
- zookeeperConnectionAddress1 = "zookeeper://127.0.0.1:2181";
- Curator5ZookeeperClient mockCurator5ZookeeperClient = mock(Curator5ZookeeperClient.class);
- mockedCurator5ZookeeperClientConstruction =
- mockConstructionWithAnswer(Curator5ZookeeperClient.class, invocationOnMock -> invocationOnMock
- .getMethod()
- .invoke(mockCurator5ZookeeperClient, invocationOnMock.getArguments()));
+ String zookeeperConnectionAddress1 = "zookeeper://127.0.0.1:2181";
+ zookeeperUrl = URL.valueOf(zookeeperConnectionAddress1 + "/service");
+
+ CuratorFrameworkFactory.Builder realBuilder = CuratorFrameworkFactory.builder();
+ CuratorFrameworkFactory.Builder spyBuilder = spy(realBuilder);
Review Comment:
see line 78: CuratorFrameworkFactory.Builder spyBuilder = CuratorFrameworkFactory.builder();
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org