You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "AlbumenJ (via GitHub)" <gi...@apache.org> on 2025/02/08 08:43:10 UTC

[PR] Try register Service Instance after export [dubbo]

AlbumenJ opened a new pull request, #15113:
URL: https://github.com/apache/dubbo/pull/15113

   ## What is the purpose of the change?
   
   
   ## 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] Try register Service Instance after export [dubbo]

Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM commented on code in PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#discussion_r1949300591


##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java:
##########
@@ -989,21 +989,21 @@ private DynamicConfiguration getDynamicConfiguration(URL connectionURL) {
      */
     private final AtomicInteger serviceRefreshState = new AtomicInteger(0);
 
-    private void registerServiceInstance() {
-        try {
-            registered = true;
+    public synchronized void registerServiceInstance() {

Review Comment:
   `checkState` already uses synchronized, so consider deleting it here.
    



-- 
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] Try register Service Instance after export [dubbo]

Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2646735694

   @CrazyHZM @chickenlj 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] Try register Service Instance after export [dubbo]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2644731517

   ## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/15113?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `80.00000%` with `2 lines` in your changes missing coverage. Please review.
   > Project coverage is 29.22%. Comparing base [(`22395f8`)](https://app.codecov.io/gh/apache/dubbo/commit/22395f890c03d775751b0ddbef2a46cae1969ee3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`0994d6e`)](https://app.codecov.io/gh/apache/dubbo/commit/0994d6e1bdf527ff5fa8cdeb4fcde6b873d3af8f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 27 commits behind head on 3.3.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/15113?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...ubbo/config/deploy/DefaultApplicationDeployer.java](https://app.codecov.io/gh/apache/dubbo/pull/15113?src=pr&el=tree&filepath=dubbo-config%2Fdubbo-config-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fconfig%2Fdeploy%2FDefaultApplicationDeployer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9kZXBsb3kvRGVmYXVsdEFwcGxpY2F0aW9uRGVwbG95ZXIuamF2YQ==) | 71.42% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/15113?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 (22395f8) and HEAD (0994d6e). Click for more details.
   > 
   > <details><summary>HEAD has 2 uploads less than BASE</summary>
   >
   >| Flag | BASE (22395f8) | HEAD (0994d6e) |
   >|------|------|------|
   >|integration-tests|1|0|
   >|unit-tests|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##                3.3   #15113       +/-   ##
   =============================================
   - Coverage     60.73%   29.22%   -31.52%     
   + Complexity    10861     8876     -1985     
   =============================================
     Files          1882     1882               
     Lines         86007    86013        +6     
     Branches      12886    12886               
   =============================================
   - Hits          52236    25135    -27101     
   - Misses        28326    56901    +28575     
   + Partials       5445     3977     -1468     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/dubbo/pull/15113/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [integration-tests](https://app.codecov.io/gh/apache/dubbo/pull/15113/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [samples-tests](https://app.codecov.io/gh/apache/dubbo/pull/15113/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.22% <80.00%> (+0.03%)` | :arrow_up: |
   | [unit-tests](https://app.codecov.io/gh/apache/dubbo/pull/15113/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/15113?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).
   


-- 
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] Try register Service Instance after export [dubbo]

Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2683829452

   > Maybe we should enhance the UT
   
   #15164  might solve it.


-- 
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] Try register Service Instance after export [dubbo]

Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2677284543

   > `Build and Test Scheduled On 3.3` https://github.com/apache/dubbo/actions/runs/13269479987/job/37046175125 which just runned after this PR merging into 3.3:
   > 
   > ```
   > 2025-02-11T18:39:00.4118796Z [ERROR] Errors: 
   > 2025-02-11T18:39:00.4124586Z [ERROR]   ServiceConfigTest.testMethodConfigWithConfiguredArgumentType:492 » Rpc Fail to start server(url: dubbo://10.1.0.109:20880/org.apache.dubbo.config.api.DemoService?anyhost=true&application=app&background=false&bind.ip=10.1.0.109&bind.port=20880&channel.readonly.sent=true&codec=dubbo&deprecated=false&dubbo=2.0.2&dynamic=true&executor-management-mode=isolation&export=true&file-cache=true&generic=false&heartbeat=60000&interface=org.apache.dubbo.config.api.DemoService&methods=callInnerClass,echo,getBox,getUsers,sayName,throwDemoException&pid=236894&register=false&release=3.3.4-SNAPSHOT&sayName.0.callback=false&sayName.return=true&sayName.sent=true&side=provider&timestamp=1739299104130) Failed to bind NettyServer on /0.0.0.0:20880, cause: Address already in use
   > 2025-02-11T18:39:00.4131440Z [ERROR]   MultipleRegistryCenterServiceDiscoveryRegistryIntegrationTest.integrate:152 » Runtime org.apache.dubbo.rpc.RpcException: Fail to start server(url: dubbo://10.1.0.109:20880/org.apache.dubbo.config.integration.multiple.servicediscoveryregistry.MultipleRegistryCenterServiceDiscoveryRegistryService?anyhost=true&application=multiple-registry-center-provider-for-service-discovery-registry-protocol&async=false&background=false&bind.ip=10.1.0.109&bind.port=20880&channel.readonly.sent=true&codec=dubbo&deprecated=false&dubbo=2.0.2&dynamic=true&executor-management-mode=isolation&file-cache=true&generic=false&heartbeat=60000&interface=org.apache.dubbo.config.integration.multiple.servicediscoveryregistry.MultipleRegistryCenterServiceDiscoveryRegistryService&methods=hello&pid=236894&prefer.serialization=hessian2,fastjson2&release=3.3.4-SNAPSHOT&service-name-mapping=true&side=provider&timestamp=1739299132226) Failed to bind NettyServer on /0.0.0.0:20880,
  cause: Address already in use
   > ```
   
   Maybe we should enhance the UT


-- 
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] Try register Service Instance after export [dubbo]

Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#discussion_r1950160831


##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java:
##########
@@ -989,21 +989,21 @@ private DynamicConfiguration getDynamicConfiguration(URL connectionURL) {
      */
     private final AtomicInteger serviceRefreshState = new AtomicInteger(0);
 
-    private void registerServiceInstance() {
-        try {
-            registered = true;
+    public synchronized void registerServiceInstance() {

Review Comment:
   This might be called by `ServiceConfig` and `ServiceConfig` can be parallel.



-- 
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] Try register Service Instance after export [dubbo]

Posted by "CrazyHZM (via GitHub)" <gi...@apache.org>.
CrazyHZM merged PR #15113:
URL: https://github.com/apache/dubbo/pull/15113


-- 
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] Try register Service Instance after export [dubbo]

Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2686536824

   since github actions might run multiple tests concurrently, i doubt those tests in ReferenceConfigTest,ServiceConfigTest,DubboBootstrapTest,MultipleRegistryCenterServiceDiscoveryRegistryIntegrationTest could be ran simultaneously as they export the same dubbo port.
   


-- 
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] Try register Service Instance after export [dubbo]

Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2675864986

   ```Build and Test Scheduled On 3.3``` https://github.com/apache/dubbo/actions/runs/13269479987/job/37046175125 which just runned after this PR merging into 3.3:
   ```
   2025-02-11T18:39:00.4118796Z [ERROR] Errors: 
   2025-02-11T18:39:00.4124586Z [ERROR]   ServiceConfigTest.testMethodConfigWithConfiguredArgumentType:492 » Rpc Fail to start server(url: dubbo://10.1.0.109:20880/org.apache.dubbo.config.api.DemoService?anyhost=true&application=app&background=false&bind.ip=10.1.0.109&bind.port=20880&channel.readonly.sent=true&codec=dubbo&deprecated=false&dubbo=2.0.2&dynamic=true&executor-management-mode=isolation&export=true&file-cache=true&generic=false&heartbeat=60000&interface=org.apache.dubbo.config.api.DemoService&methods=callInnerClass,echo,getBox,getUsers,sayName,throwDemoException&pid=236894&register=false&release=3.3.4-SNAPSHOT&sayName.0.callback=false&sayName.return=true&sayName.sent=true&side=provider&timestamp=1739299104130) Failed to bind NettyServer on /0.0.0.0:20880, cause: Address already in use
   2025-02-11T18:39:00.4131440Z [ERROR]   MultipleRegistryCenterServiceDiscoveryRegistryIntegrationTest.integrate:152 » Runtime org.apache.dubbo.rpc.RpcException: Fail to start server(url: dubbo://10.1.0.109:20880/org.apache.dubbo.config.integration.multiple.servicediscoveryregistry.MultipleRegistryCenterServiceDiscoveryRegistryService?anyhost=true&application=multiple-registry-center-provider-for-service-discovery-registry-protocol&async=false&background=false&bind.ip=10.1.0.109&bind.port=20880&channel.readonly.sent=true&codec=dubbo&deprecated=false&dubbo=2.0.2&dynamic=true&executor-management-mode=isolation&file-cache=true&generic=false&heartbeat=60000&interface=org.apache.dubbo.config.integration.multiple.servicediscoveryregistry.MultipleRegistryCenterServiceDiscoveryRegistryService&methods=hello&pid=236894&prefer.serialization=hessian2,fastjson2&release=3.3.4-SNAPSHOT&service-name-mapping=true&side=provider&timestamp=1739299132226) Failed to bind NettyServer on /0.0.0.0:20880, c
 ause: Address already in use
   ```
   


-- 
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] Try register Service Instance after export [dubbo]

Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #15113:
URL: https://github.com/apache/dubbo/pull/15113#issuecomment-2675857473

   ReferenceConfigTest,ServiceConfigTest,DubboBootstrapTest,MultipleRegistryCenterServiceDiscoveryRegistryIntegrationTest of dubbo-config-api might be affected as sometimes RpcException with ```Address already in use``` will be thrown.
   see https://github.com/apache/dubbo/actions/runs/13462589128/job/37621771946
   ```
   2025-02-21T18:35:26.8596499Z [ERROR]   ReferenceConfigTest.testClassLoader:1164 » Rpc Fail to start server(url: dubbo://10.1.0.216:20880/org.apache.dubbo.config.api.DemoService?anyhost=true&application=Test&background=false&bind.ip=10.1.0.216&bind.port=20880&channel.readonly.sent=true&codec=dubbo&deprecated=false&dubbo=2.0.2&dynamic=true&executor-management-mode=isolation&file-cache=true&generic=false&heartbeat=60000&interface=org.apache.dubbo.config.api.DemoService&methods=callInnerClass,echo,getBox,getUsers,sayName,throwDemoException&pid=236855&register=false&release=3.3.4-SNAPSHOT&side=provider&timestamp=1740162893508) Failed to bind NettyServer on /0.0.0.0:20880, cause: Address already in use
   ...
   ```
   
   
   


-- 
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