You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2021/09/14 11:53:22 UTC

[GitHub] [servicecomb-java-chassis] maxucheng0 opened a new pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions (#2574)

maxucheng0 opened a new pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578


   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean install -Pit` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   


-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] liubao68 closed pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
liubao68 closed pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578


   


-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#discussion_r708220320



##########
File path: foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/consumer/MicroserviceManager.java
##########
@@ -60,8 +60,8 @@ public MicroserviceVersions getOrCreateMicroserviceVersions(String microserviceN
         microserviceVersions = versionsByName.get(microserviceName);
         if (microserviceVersions == null) {
           microserviceVersions = new MicroserviceVersions(appManager, appId, microserviceName);
-          microserviceVersions.pullInstances();
           versionsByName.put(microserviceName, microserviceVersions);
+          microserviceVersions.pullInstances();

Review comment:
       Maybe you can add a synchronized to `pullInstances`  and `onMicroserviceInstanceChanged` can solve this problem. 
   And your modification seems not correct and may cause some other concurent problems and add add a cyclic dependency between foundation-registry and registry-service-center.
   
   ```
     public void pullInstances() {
     synchronized (lock) {
       for (MicroserviceVersions microserviceVersions : versionsByName.values()) {
         microserviceVersions.pullInstances();
         tryRemoveInvalidMicroservice(microserviceVersions);
       }
     }
     }
     public void onMicroserviceInstanceChanged(MicroserviceInstanceChangedEvent changedEvent) {
       synchronized (lock) {
       for (MicroserviceVersions microserviceVersions : versionsByName.values()) {
         microserviceVersions.onMicroserviceInstanceChanged(changedEvent);
         tryRemoveInvalidMicroservice(microserviceVersions);
       }
      }
     }
   ```
   




-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] liubao68 commented on a change in pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#discussion_r708220320



##########
File path: foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/consumer/MicroserviceManager.java
##########
@@ -60,8 +60,8 @@ public MicroserviceVersions getOrCreateMicroserviceVersions(String microserviceN
         microserviceVersions = versionsByName.get(microserviceName);
         if (microserviceVersions == null) {
           microserviceVersions = new MicroserviceVersions(appManager, appId, microserviceName);
-          microserviceVersions.pullInstances();
           versionsByName.put(microserviceName, microserviceVersions);
+          microserviceVersions.pullInstances();

Review comment:
       Maybe you can add a synchronized to `pullInstances`  and `onMicroserviceInstanceChanged` can solve this problem. 
   And your modification seems not correct and may cause some other concurent problems and add add a cyclic dependency between foundation-registry and registry-service-center.
   
   ```
     public void pullInstances() {
       for (MicroserviceVersions microserviceVersions : versionsByName.values()) {
         microserviceVersions.pullInstances();
         tryRemoveInvalidMicroservice(microserviceVersions);
       }
     }
     public void onMicroserviceInstanceChanged(MicroserviceInstanceChangedEvent changedEvent) {
       for (MicroserviceVersions microserviceVersions : versionsByName.values()) {
         microserviceVersions.onMicroserviceInstanceChanged(changedEvent);
         tryRemoveInvalidMicroservice(microserviceVersions);
       }
     }
   ```
   




-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] maxucheng0 commented on a change in pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
maxucheng0 commented on a change in pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#discussion_r708768989



##########
File path: foundations/foundation-registry/src/main/java/org/apache/servicecomb/registry/consumer/MicroserviceManager.java
##########
@@ -60,8 +60,8 @@ public MicroserviceVersions getOrCreateMicroserviceVersions(String microserviceN
         microserviceVersions = versionsByName.get(microserviceName);
         if (microserviceVersions == null) {
           microserviceVersions = new MicroserviceVersions(appManager, appId, microserviceName);
-          microserviceVersions.pullInstances();
           versionsByName.put(microserviceName, microserviceVersions);
+          microserviceVersions.pullInstances();

Review comment:
       好的,稍后我修改并验证下




-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] liubao68 commented on pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
liubao68 commented on pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#issuecomment-922237327


   fixed in https://github.com/apache/servicecomb-java-chassis/pull/2593


-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] maxucheng0 commented on pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
maxucheng0 commented on pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#issuecomment-919625976


   > BTW, I see you add an JIRA issue [SCB-2337], but I can't find this in JIRA https://issues.apache.org/jira/projects/SCB/versions/12350513
   > 
   > Do you have an JIRA account or have mistake in the issue ID?
   > I can add you to JIRA group if you do not have one, see https://servicecomb.apache.org/cn/developers/use-jira/
   
   已创建


-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [servicecomb-java-chassis] liubao68 commented on pull request #2578: [SCB-2337] Add timing verification for MicroserviceVersions

Posted by GitBox <gi...@apache.org>.
liubao68 commented on pull request #2578:
URL: https://github.com/apache/servicecomb-java-chassis/pull/2578#issuecomment-919118170


   BTW, I see you add an JIRA issue [SCB-2337], but I can't find this in JIRA https://issues.apache.org/jira/projects/SCB/versions/12350513 
   
   Do you have an JIRA account or have mistake in the issue ID? 
   I can add you to JIRA group if you do not have one, see https://servicecomb.apache.org/cn/developers/use-jira/
   


-- 
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: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org