You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "ghostly72 (via GitHub)" <gi...@apache.org> on 2025/12/15 10:42:28 UTC
[PR] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
ghostly72 opened a new pull request, #15873:
URL: https://github.com/apache/dubbo/pull/15873
Currently, Dubbo’s registry layer writes provider information to a local cache file, but it does not read that cache when the registry center becomes unavailable. So, consumers fail to discover providers during temporary registry outages, even though the information already exists locally.
This PR introduces a fallback mechanism so that consumers can still find providers using the last known data in the local cache.
Added some code in org.apache.dubbo.registry.support.AbstractRegistry to:
Load provider URLs from the local cache file when the registry cannot be reached.
Immediately notify registered listeners with cached data so consumers remain functional.
The fallback is triggered only when the registry is unavailable or not yet ready.
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3663472246
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15873?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 `46.66667%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 32.14%. Comparing base ([`efed26c`](https://app.codecov.io/gh/apache/dubbo/commit/efed26caac8f728a0c16ad8b7f8aefb16adbdfec?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`db5922d`](https://app.codecov.io/gh/apache/dubbo/commit/db5922d56fc5c6dc2d696bb588460ddd5a2e72a0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 20 commits behind head on 3.3.
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15873?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...pache/dubbo/registry/support/AbstractRegistry.java](https://app.codecov.io/gh/apache/dubbo/pull/15873?src=pr&el=tree&filepath=dubbo-registry%2Fdubbo-registry-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fregistry%2Fsupport%2FAbstractRegistry.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L0Fic3RyYWN0UmVnaXN0cnkuamF2YQ==) | 46.66% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15873?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 (efed26c) and HEAD (db5922d). Click for more details.
>
> <details><summary>HEAD has 7 uploads less than BASE</summary>
>
>| Flag | BASE (efed26c) | HEAD (db5922d) |
>|------|------|------|
>|integration-tests-java21|1|0|
>|integration-tests-java8|1|0|
>|unit-tests-java8|1|0|
>|unit-tests-java25|1|0|
>|unit-tests-java11|1|0|
>|unit-tests-java21|1|0|
>|unit-tests-java17|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.3 #15873 +/- ##
=============================================
- Coverage 60.77% 32.14% -28.63%
+ Complexity 11710 10155 -1555
=============================================
Files 1938 1937 -1
Lines 88692 88687 -5
Branches 13387 13307 -80
=============================================
- Hits 53903 28510 -25393
- Misses 29260 56017 +26757
+ Partials 5529 4160 -1369
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/15873/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/15873/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/15873/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/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.07% <46.66%> (+0.09%)` | :arrow_up: |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.78% <46.66%> (+0.08%)` | :arrow_up: |
| [unit-tests-java11](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java17](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java25](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/15873/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/15873?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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on code in PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#discussion_r2635338785
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java:
##########
@@ -456,6 +463,24 @@ public void subscribe(URL url, NotifyListener listener) {
Set<NotifyListener> listeners =
ConcurrentHashMapUtils.computeIfAbsent(subscribed, url, n -> new ConcurrentHashSet<>());
listeners.add(listener);
+ if (localCacheEnabled) {
+ try {
+ List<URL> cached = getCacheUrls(url);
+ if (CollectionUtils.isNotEmpty(cached)) {
+ listener.notify(cached);
+ logger.info("Registry unavailable or not yet ready. Loaded cached URLs for " + url.getServiceKey()
Review Comment:
```
if(localCacheEnabled){
try{
List<URL> cached=getCacheUrls(url);
if(CollectionUtils.isNotEmpty(cached)){
listener.notify(cached);
logger.info("Registry unavailable or not yet ready. Loaded cached URLs for "+url.getServiceKey()+" : "+cached.size());
}
}catch(Throwable t){
logger.warn(INTERNAL_ERROR,"failed to load local cache","","Failed to load chached registry data for "+url,t);
throw t;
}
}
```
from what i understood, i think cached list being not empty simply refers to some cached urls being available and the functionality checking for the unavailability of registry is taken care of by
` if(localcacheenbled)`
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3685296409
> run `mvn spotless:apply` to fix codes format of your pr.
yup. done
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3739079634
so to detect only the case of registry failure, can we add another flag like isRegistryDown in abstract registry?
im thinking of maybr adding:
```
private volatile boolean registryDown = false;
protected boolean isRegistryDown() {
return registryDown;
}
protected void setRegistryDown(boolean down) {
this.registryDown = down;
}
```
and
```
private boolean isConnectionException(Throwable t) {
return t instanceof java.io.IOException
|| t instanceof java.net.ConnectException
|| t instanceof java.net.SocketTimeoutException
|| (t.getMessage() != null && t.getMessage().contains("Connection refused"));
// we can add other exceptions that correspond to the registry failure or unavailability scenario as well
}
```
inside abstract registry.
other registry implementatoins ultimately extend this.
so in every registry , we'll have code block similar to
```
try{
// register/unregister/subscriber/unsubscribe to registry
}catch{
// msg saying failed to connect due to some error
// here we can call isConnectionException and check if we should set the flag or not accordingly
```
and then finally inside abstractregistry's code
we can have
```
if(!isRegistryDown) enable local cache ```
thoughts ?
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on code in PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#discussion_r2640304202
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java:
##########
@@ -456,6 +463,24 @@ public void subscribe(URL url, NotifyListener listener) {
Set<NotifyListener> listeners =
ConcurrentHashMapUtils.computeIfAbsent(subscribed, url, n -> new ConcurrentHashSet<>());
listeners.add(listener);
+ if (localCacheEnabled) {
+ try {
+ List<URL> cached = getCacheUrls(url);
+ if (CollectionUtils.isNotEmpty(cached)) {
+ listener.notify(cached);
+ logger.info("Registry unavailable or not yet ready. Loaded cached URLs for " + url.getServiceKey()
Review Comment:
i just added the `!isavailable` parameter as well
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on code in PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#discussion_r2635355483
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java:
##########
@@ -456,6 +463,24 @@ public void subscribe(URL url, NotifyListener listener) {
Set<NotifyListener> listeners =
ConcurrentHashMapUtils.computeIfAbsent(subscribed, url, n -> new ConcurrentHashSet<>());
listeners.add(listener);
+ if (localCacheEnabled) {
+ try {
+ List<URL> cached = getCacheUrls(url);
+ if (CollectionUtils.isNotEmpty(cached)) {
+ listener.notify(cached);
+ logger.info("Registry unavailable or not yet ready. Loaded cached URLs for " + url.getServiceKey()
Review Comment:
and now looking at it i think instead of
`if(localCacheEnabled) `
it should be
`if(localCacheEnabled and !isAvailable())` ?
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3684798856
run ```mvn spotless:apply``` to fix codes format of your 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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3663351412
The code format needs to comply with regulatory requirements, you should run mvn spotless:apply to format your codes before submit the 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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3667964304
> Assertions.assertFalse(abstractRegistry.getSubscribed().containsKey(listener));
> but shouldnt it be containsKey(testUrl) instead of checking if it contains listener?
yes, it should be ```containsKey(testUrl)```
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#discussion_r2633219571
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java:
##########
@@ -456,6 +463,24 @@ public void subscribe(URL url, NotifyListener listener) {
Set<NotifyListener> listeners =
ConcurrentHashMapUtils.computeIfAbsent(subscribed, url, n -> new ConcurrentHashSet<>());
listeners.add(listener);
+ if (localCacheEnabled) {
+ try {
+ List<URL> cached = getCacheUrls(url);
+ if (CollectionUtils.isNotEmpty(cached)) {
+ listener.notify(cached);
+ logger.info("Registry unavailable or not yet ready. Loaded cached URLs for " + url.getServiceKey()
Review Comment:
why ```not-empty cached urls``` means ```Registry unavailable or not yet ready``` ?
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3739073769
hi
right now, in the pr i created for this, my code goes like (inside abstractregistry.java)
```
if(localCacheEnabled && !isAvailable){
try{
List<URL> cached=getCacheUrls(url);
if(CollectionUtils.isNotEmpty(cached)){
listener.notify(cached);
logger.info("Registry unavailable or not yet ready. Loaded cached URLs for "+url.getServiceKey()+" : "+cached.size());
}
}catch(Throwable t){
logger.warn(INTERNAL_ERROR,"failed to load local cache","","Failed to load chached registry data for "+url,t);
throw t;
}
}
```
but this fails tests because i think isAvailable() returns false not only when the registry crashes, but also when:
-the registry hasn’t connected yet (say during startup)
-the client object is null (like in tests or mocks)
-the registry was shut down gracefully
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "ghostly72 (via GitHub)" <gi...@apache.org>.
ghostly72 commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3665218845
> `AbstractRegistryTest` should be fixed if apply this pr,
>
> ```
> Error: ERROR] org.apache.dubbo.registry.support.AbstractRegistryTest.allTest -- Time elapsed: 0.008 s <<< FAILURE!
> 60514-org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
> 60515- at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
> 60516- at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
> 60517- at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
> 60518- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
> 60519- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
> 60520- at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
> 60521- at org.apache.dubbo.registry.support.AbstractRegistryTest.allTest(AbstractRegistryTest.java:516)
> 60522- at java.lang.reflect.Method.invoke(Method.java:498)
> 60523- at java.util.ArrayList.forEach(ArrayList.java:1259)
> 60524- at java.util.ArrayList.forEach(ArrayList.java:1259)
> 60525-
> Error: ERROR] org.apache.dubbo.registry.support.AbstractRegistryTest.lookupTest -- Time elapsed: 0.005 s <<< FAILURE!
> 60527-org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
> 60528- at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
> 60529- at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
> 60530- at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
> 60531- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
> 60532- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
> 60533- at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
> 60534- at org.apache.dubbo.registry.support.AbstractRegistryTest.lookupTest(AbstractRegistryTest.java:485)
> 60535- at java.lang.reflect.Method.invoke(Method.java:498)
> 60536- at java.util.ArrayList.forEach(ArrayList.java:1259)
> 60537- at java.util.ArrayList.forEach(ArrayList.java:1259)
> ```
yea i saw the test file and found that lookuptest expects to return a list that does not contain testUrl. but the modified subscribe() method, notifies from cache immediately when local cache data is available. That means the cache is loaded and lookup() might return testUrl, making the assertion fail.
so should i try and make some changes in the test file or else should i modify my code?
also in the same file in `alltest()`, there is a line checking (line 520)
```abstractRegistry.unsubscribe(testUrl, listener);
Assertions.assertFalse(abstractRegistry.getSubscribed().containsKey(listener));
```
but shouldnt it be `containsKey(testUrl)` instead of checking if it contains listener?
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#issuecomment-3663454383
```AbstractRegistryTest``` should be fixed if apply this pr,
```
Error: ERROR] org.apache.dubbo.registry.support.AbstractRegistryTest.allTest -- Time elapsed: 0.008 s <<< FAILURE!
60514-org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
60515- at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
60516- at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
60517- at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
60518- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
60519- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
60520- at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
60521- at org.apache.dubbo.registry.support.AbstractRegistryTest.allTest(AbstractRegistryTest.java:516)
60522- at java.lang.reflect.Method.invoke(Method.java:498)
60523- at java.util.ArrayList.forEach(ArrayList.java:1259)
60524- at java.util.ArrayList.forEach(ArrayList.java:1259)
60525-
Error: ERROR] org.apache.dubbo.registry.support.AbstractRegistryTest.lookupTest -- Time elapsed: 0.005 s <<< FAILURE!
60527-org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
60528- at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
60529- at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
60530- at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
60531- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
60532- at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
60533- at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
60534- at org.apache.dubbo.registry.support.AbstractRegistryTest.lookupTest(AbstractRegistryTest.java:485)
60535- at java.lang.reflect.Method.invoke(Method.java:498)
60536- at java.util.ArrayList.forEach(ArrayList.java:1259)
60537- at java.util.ArrayList.forEach(ArrayList.java:1259)
```
--
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] providing local cache support for service discovery when registry is unavailable #13856 [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #15873:
URL: https://github.com/apache/dubbo/pull/15873#discussion_r2638548062
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/AbstractRegistry.java:
##########
@@ -456,6 +463,24 @@ public void subscribe(URL url, NotifyListener listener) {
Set<NotifyListener> listeners =
ConcurrentHashMapUtils.computeIfAbsent(subscribed, url, n -> new ConcurrentHashSet<>());
listeners.add(listener);
+ if (localCacheEnabled) {
+ try {
+ List<URL> cached = getCacheUrls(url);
+ if (CollectionUtils.isNotEmpty(cached)) {
+ listener.notify(cached);
+ logger.info("Registry unavailable or not yet ready. Loaded cached URLs for " + url.getServiceKey()
Review Comment:
as for me, it might be better than changing the default behavior 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