[Dubbo-4793]make RegistryDirectory can refresh the invokers when providers number become 0 when using nacos registry #4802
Conversation
… become 0 when using nacos registry
… become 0 when using nacos registry
Codecov Report
@@ Coverage Diff @@
## master #4802 +/- ##
===========================================
- Coverage 63.92% 63.9% -0.02%
Complexity 451 451
===========================================
Files 769 769
Lines 33171 33179 +8
Branches 5229 5232 +3
===========================================
Hits 21204 21204
- Misses 9547 9549 +2
- Partials 2420 2426 +6
Continue to review full report at Codecov.
|
… become 0 when using nacos registry
tswstarplanet
force-pushed
the
nacos_registry
branch
from
9925f7c to
e78ec79
Compare
| URL url = buildURL(instance); | ||
| if (UrlUtils.isMatch(consumerURL, url)) { | ||
| urls.add(url); | ||
| if (instances != null && !instances.isEmpty()) { |
There was a problem hiding this comment.
Maybe we don't need to check whether instances is empty
There was a problem hiding this comment.
For String or Collection, I think not only null, but also size check are needed because the for each will create useless iterator object.
| // Healthy Instances | ||
| filterHealthyInstances(healthyInstances); | ||
| List<URL> urls = buildURLs(url, healthyInstances); | ||
| if (healthyInstances.size() > 0) { |
There was a problem hiding this comment.
If healthyInstances is empty, this still works? Not sure, could u pls check it.
I believe we should make our code as clean as possible :)
There was a problem hiding this comment.
Same as above. I think it's a fail-fast term.
| private List<URL> buildURLs(URL consumerURL, Collection<Instance> instances) { | ||
| if (instances.isEmpty()) { | ||
| return Collections.emptyList(); | ||
| private List<URL> toUrlWithEmpty(URL consumerURL, Collection<Instance> instances) { |
There was a problem hiding this comment.
Last nitpick, what about changing toUrlWithEmpty to toUrlsWhenEmpty? (just
one suggestion, there should be another better method name to make it clear)
I know we use toUrlWithEmpty in zookeeperRegistry.class, but I found the name may bring us a little confusion :)
There was a problem hiding this comment.
The name tells that the protocol will be set "empty"
|
|
What is the purpose of the change
fix #4793
Brief changelog
NacosRegistry.java
fix issue 4793, add toUrlWithEmpty emthod like ZookeeperRegistry.java, when the provider number become 0, generate an url with empty protocol.
Verifying this change
has been test
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false&mvn clean test-compile failsafe:integration-testto make sure unit-test and integration-test pass.