apache
Feat support check for config-center and metadata-center
What is the purpose of the change?
resolve #15623 , to support the options check=false for registry-center config-center and metadata-center. There contains the content of PR #15594. The specific changes are as follows:
config-center
- When use the registry-center address as config-center or metadata-center adddress, there should assign the value of option
registry.checkto the config-center or metadata-center config option.
related change files:
- org.apache.dubbo.config.deploy.DefaultApplicationDeployer
- Add method
boolean isAvailable()onDynamicConfiguration, to check the connection status.
related change files:
- org.apache.dubbo.common.config.configcenter.DynamicConfiguration
- org.apache.dubbo.configcenter.support.nacos.NacosDynamicConfiguration
- org.apache.dubbo.configcenter.support.zookeeper.ZookeeperDynamicConfiguration
- org.apache.dubbo.common.config.configcenter.wrapper.CompositeDynamicConfiguration
~~3. Cache the operation of register config item listener when the connection is not available.~~
~~related change files:~~
~~- org.apache.dubbo.common.config.configcenter.wrapper.FailbackDynamicConfiguration~~ ~~- org.apache.dubbo.configcenter.support.nacos.NacosDynamicConfigurationFactory~~ ~~- org.apache.dubbo.configcenter.support.zookeeper.ZookeeperDynamicConfigurationFactory~~
the config-center item listener is support async add.
service-name-mapping
- Refectory the reporting process of
ServiceNameMapping, let it support failure retries.
related change files:
- org.apache.dubbo.config.ServiceConfig
- org.apache.dubbo.metadata.AbstractServiceNameMapping
- org.apache.dubbo.metadata.ServiceNameMapping
metadata
- Add method
boolean isAvailable()onMetadataReport, to check the connection status.
related change files:
- org.apache.dubbo.metadata.report.MetadataReport
- org.apache.dubbo.metadata.report.support.AbstractMetadataReport
- org.apache.dubbo.metadata.store.zookeeper.ZookeeperMetadataReport
- org.apache.dubbo.metadata.store.nacos.NacosMetadataReport
- Support the option of
check=false
related change files:
- org.apache.dubbo.metadata.report.support.AbstractMetadataReportFactory
- org.apache.dubbo.metadata.store.zookeeper.ZookeeperMetadataReport
- org.apache.dubbo.metadata.store.nacos.NacosMetadataReport
service-discovery
- Add field
boolean reportedto record whether the metadata reporting is successful or not, Add fieldboolean registeredto record where the service-instance register is successful or not.
related change files:
- org.apache.dubbo.metadata.MetadataInfo
- org.apache.dubbo.registry.client.ServiceInstance
- org.apache.dubbo.registry.client.DefaultServiceInstance
- Add failure retry logic for metadata reporting and service-instance registration.
related change files:
- org.apache.dubbo.registry.client.AbstractServiceDiscovery
consumer
Support the consumer check option, if it is false, the subscribe fail will retry by FailbackRegistry.
related change files:
- org.apache.dubbo.registry.client.ServiceDiscoveryRegistry
Checklist
- [x] Make sure there is a GitHub_issue 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 project.
- [x] Make sure gitHub actions can pass. Why the workflow is failing and how to fix it?
Codecov Report
:x: Patch coverage is 66.80328% with 81 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 60.77%. Comparing base (f284fab) to head (f758db1).
Additional details and impacted files
@@ Coverage Diff @@
## 3.3 #15639 +/- ##
==========================================
Coverage 60.76% 60.77%
- Complexity 11712 11716 +4
==========================================
Files 1938 1939 +1
Lines 88646 88778 +132
Branches 13379 13398 +19
==========================================
+ Hits 53866 53951 +85
- Misses 29257 29274 +17
- Partials 5523 5553 +30
| Flag | Coverage Δ | |
|---|---|---|
| integration-tests-java21 | 32.41% <44.67%> (-0.04%) |
:arrow_down: |
| integration-tests-java8 | 32.48% <44.67%> (+0.04%) |
:arrow_up: |
| samples-tests-java21 | 32.04% <35.24%> (+<0.01%) |
:arrow_up: |
| samples-tests-java8 | 29.71% <32.78%> (+0.11%) |
:arrow_up: |
| unit-tests-java11 | 59.08% <63.11%> (-0.01%) |
:arrow_down: |
| unit-tests-java17 | 58.57% <63.11%> (+0.02%) |
:arrow_up: |
| unit-tests-java21 | 58.53% <63.11%> (-0.04%) |
:arrow_down: |
| unit-tests-java25 | 58.50% <63.11%> (+<0.01%) |
:arrow_up: |
| unit-tests-java8 | 59.04% <63.11%> (-0.05%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
In addition to adding the isAvailable method, the following sections describe some logical adjustments to support the failure retry strategy.
service-name-mapping
consumer
The service-name-mapping listener support local cache, no need to retry.
provider
If serviceNameMapping.map(url) return false, it will retry by FrameworkExecutorRepository schedule executor.
https://github.com/apache/dubbo/blob/d45ce97c489bc096102533b7a87ef80e3184d2e9/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java#L426-L469
But the value of variable result is determined by the report result of the last metadata-report client.
Maybe there should retry every report failed metadata-report client.
https://github.com/apache/dubbo/blob/d45ce97c489bc096102533b7a87ef80e3184d2e9/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/MetadataServiceNameMapping.java#L94-L156
In order to support failed retries, I have made the following changes:
add the registerServiceAppMapping method, calculate whether the appName is exists in the metadata-center, if it doesn't exists report it.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/AbstractServiceNameMapping.java#L144-L171
define the abstract method boolean doMap(MetadataReport metadataReport, URL url) to execute the reporting logic of appName.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/AbstractServiceNameMapping.java#L142
Here I implement the doMap method, retained the original failed retry logic.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/metadata/MetadataServiceNameMapping.java#L62-L92
The boolean map(URL url) method move to AbstractServiceNameMapping, it will add the report failed metadata-report client and service to the retry task.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/AbstractServiceNameMapping.java#L114-L139
Add a new class MetadataReportRetryTask (some unit test waiting for add), it will schedule on fixed rate, retry task use FrameworkExecutorRepository share thread pool.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/report/MetadataReportRetryTask.java#L42-L57
It will replace the original code https://github.com/apache/dubbo/blob/d45ce97c489bc096102533b7a87ef80e3184d2e9/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java#L462-L468
service-discovery
consumer
When consumer subscribe service, it will get service instance by service-discovery, if the service-discovery connection is unavailable, there will throw an exception.
https://github.com/apache/dubbo/blob/d45ce97c489bc096102533b7a87ef80e3184d2e9/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java#L356
If consumer.check = false, support for failed retry is required, here add retry logic by method FailbackRegistry.addFailedSubscribed().
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java#L387-L394
Due to current retry tasks calling the doSubscribe method, service-discovery subscribe process need only call subscribeURLs method no need to call from doSubscribe, so here i add a new method void addFailedSubscribed(URL url, NotifyListener listener, ThrowableAction handler) to support customize retry processing logic.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/support/FailbackRegistry.java#L138-L154
add new field ThrowableAction handler for FailedSubscribedTask, handler is the customize function. If handler is null, the retry will call doSubscribe or else customize function will called.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/retry/FailedSubscribedTask.java#L30-L49
provider
In the method 'calOrupdateInstanceRevision', it calculates whether metadata has been updated. But it will return true once, This results in the inability to retry when metadata reporting fails or when serviceInstance register or update fails.
https://github.com/apache/dubbo/blob/d45ce97c489bc096102533b7a87ef80e3184d2e9/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L379-L388
The following changes have been made to support failed retries:
If the filed serviceInstance is null, this means that the serviceInstance need to be initialized first and registered to registry-center.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L70
Add field String reportedRevision in class MetadataInfo, it records the revision of metadata that has successfully reported to metadata-center.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-metadata/dubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/MetadataInfo.java#L82
On the line 405 of method reportMetadata, preventing duplicate reporting by comparing the value of MetadataInfo.revision and MetadataInfo.reportedRevision.
On the line 418 of method reportMetadata, after the metadata is successfully reported, update the value of 'MetadataInfo.reportedRevision' to the successfully reported revision.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L404-L427
Refactored the logic of the 'calOrUpdateInstanceRevision' method. Enable it to support retry after failer.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L397-L402
When execute register() or update() method, it will clone a new ServiceInstance Object from field serviceInstance and named newServiceInstance, Then assign the revision value of the successfully reported metadata to newServiceInstance.metadata[dubbo.metadata.revision]. Once the method register() or update() is execute successfully, newServiceInstance will assign to field serviceInstance.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L170-L178
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L212-L217
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java#L132-L137
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperServiceDiscovery.java#L135-L138
There are some special cases. if the scene here is re-register, it is the same as register(). if the scene is unregister, there will assign 0 to MetadataInfo.reportedRevision, to prevent duplicate execution of update logic in calOrUpdateInstanceRevision method.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java#L365-L376
In the updateInstance or deregisterInstance method of NacosNamingServiceWrapper class, place the logic to remove the local InstanceInfo object after the successful operation on the registry-center.
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java#L204-L219
https://github.com/apache/dubbo/blob/eb728417846025108aa61fb7ae777018b9ea96af/dubbo-registry/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosNamingServiceWrapper.java#L259-L279
(To facilitate code review, I have made as few changes as possible to the original code here, and optimizations can be made later.)
@zrlw Does this PR contain too many changes that make code review inconvenient? Would it be better for me to split these changes into multiple PRs?
you should fix your error code inspect failure issue first.
open the failure link at https://github.com/apache/dubbo/actions/runs/17537062451/job/50106226140?pr=15639,
or open your local build-and-test-pr.yml and run its Run Error Code Inspecting on your local machine to find the reason and correct your codes.
you should fix your error code inspect failure issue first. open the failure link at https://github.com/apache/dubbo/actions/runs/17537062451/job/50106226140?pr=15639, or open your local
build-and-test-pr.ymland run itsRun Error Code Inspectingon your local machine to find the reason and correct your codes.
Hey, it was fixed. @zrlw
@RainYuY Hi, Could you please take some time to review this PR?
The next week.
@RainYuY Hi, Could you please take some time to review this PR?
The next week.
thanks for the update
Before I do a detailed review of this PR, I’d like to ask you to reopen PR https://github.com/apache/dubbo/pull/15594 , because we’re preparing a release soon. I’d prefer to fix the bug first and review the larger PR afterwards.