Fix: exclude RegistryConfig when fast check duplicated equivalent config#16382
Open
dslztx wants to merge 2 commits into
Open
Fix: exclude RegistryConfig when fast check duplicated equivalent config#16382dslztx wants to merge 2 commits into
dslztx wants to merge 2 commits into
Conversation
…nfig
if the dubbo consumer xml config is as follow
<dubbo:registry id="zk1" protocol="zookeeper"
address="${ZK1_ADDRESS:10.47.181.23:2181,10.47.181.24:2181,10.47.181.25:2181}"/>
<dubbo:registry id="zk2" protocol="zookeeper"
address="${ZK2_ADDRESS:10.47.181.20:2181,10.47.181.21:2181,10.47.181.22:2181}"/>
<dubbo:reference id="app1" interface="com.test.app1"
registry="zk1"/>
<dubbo:reference id="app2" interface="com.test.app2"
registry="zk2"/>
in which ZK1_ADDRESS and ZK2_ADDRESS are env variables.
what is exactly needed is that ZK1_ADDRESS and ZK2_ADDRESS may be the same or different.
when ZK1_ADDRESS and ZK2_ADDRESS are the same, current implementation will only register zk1 registy config successfully, the zk2 registry config is identified as duplication，then it throws exception `java.lang.IllegalStateException: Registry not found: zk2` when loading app2 reference config
LI123456mo
reviewed
Jul 15, 2026
Contributor
There was a problem hiding this comment.
@dslztx have gone through the issue and the PR and have really understood what you were trying to fix .Have run the tests alone to connfirm the PR requests fix it worked; maybe you can try add this test and check if it passes the flacky tests?
or running-> mvn spotless:apply
@Test
void testRegistryConfigSameAddressDifferentId() {
RegistryConfig zk1 = new RegistryConfig();
zk1.setId("zk1");
zk1.setProtocol("zookeeper");
zk1.setAddress("10.47.181.23:2181,10.47.181.24:2181,10.47.181.25:2181");
RegistryConfig zk2 = new RegistryConfig();
zk2.setId("zk2");
zk2.setProtocol("zookeeper");
zk2.setAddress("10.47.181.23:2181,10.47.181.24:2181,10.47.181.25:2181");
configManager.addConfig(zk1);
configManager.addConfig(zk2);
assertTrue(configManager.getConfig(RegistryConfig.class, "zk1").isPresent());
assertTrue(configManager.getConfig(RegistryConfig.class, "zk2").isPresent());
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #16382 +/- ##
============================================
+ Coverage 60.86% 60.90% +0.04%
- Complexity 11763 11769 +6
============================================
Files 1953 1953
Lines 89262 89263 +1
Branches 13471 13472 +1
============================================
+ Hits 54331 54368 +37
+ Misses 29329 29314 -15
+ Partials 5602 5581 -21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Author
|
@LI123456mo I have added a unit test and reformatted the code according to what you said. |
zrlw
added
status/wait for another approve
type/bug
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Dubbo configuration-loading failure caused by RegistryConfig being incorrectly treated as a duplicated equivalent config (because AbstractConfig.equals() intentionally ignores id). The change ensures multiple registry configs with the same address but different IDs can coexist, matching how XML registry="..." references resolve configs by id.
Changes:
- Skip the “equivalent config” fast-duplicate check for
RegistryConfigto avoid dropping a second registry that differs only byid. - Skip the value-based duplicated-config lookup for
RegistryConfiginsideaddIfAbsent, ensuring both registry IDs remain registered. - Add a regression test asserting two
RegistryConfigentries with the same address but different IDs are both present.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dubbo-common/src/main/java/org/apache/dubbo/config/context/AbstractConfigManager.java |
Adjusts deduplication logic so RegistryConfig instances aren’t collapsed by value-equality (which ignores id). |
dubbo-common/src/test/java/org/apache/dubbo/config/context/AbstractConfigManagerTest.java |
Adds a regression test covering same-address/different-id registry configs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+216
to
222
| if (!(config instanceof RegistryConfig)) { | ||
| // find by value | ||
| Optional<C> prevConfig = findDuplicatedConfig(configsMap, config); | ||
| if (prevConfig.isPresent()) { | ||
| return prevConfig.get(); | ||
| } | ||
| } |
Comment on lines
+19
to
+41
| import org.apache.dubbo.config.RegistryConfig; | ||
| import org.apache.dubbo.config.context.ConfigManagerTest.TestPreferSerializationProvider; | ||
| import org.apache.dubbo.rpc.model.ApplicationModel; | ||
| import org.apache.dubbo.rpc.model.FrameworkModel; | ||
|
|
||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| class AbstractConfigManagerTest { | ||
|
|
||
| private ConfigManager configManager; | ||
| private ModuleConfigManager moduleConfigManager; | ||
|
|
||
| @BeforeEach | ||
| public void init() { | ||
| ApplicationModel.defaultModel().destroy(); | ||
| ApplicationModel applicationModel = ApplicationModel.defaultModel(); | ||
| configManager = applicationModel.getApplicationConfigManager(); | ||
| moduleConfigManager = applicationModel.getDefaultModule().getConfigManager(); | ||
| FrameworkModel.defaultModel().getBeanFactory().registerBean(TestPreferSerializationProvider.class); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #16381
This PR addresses the dubbo load exception which is triggered by incorrect registry config deduplication