[GitHub] [dubbo] chickenlj commented on a diff in pull request #11223: Enhance service discovery update interval

[GitBox <gi...@apache.org>]

chickenlj commented on code in PR #11223:
URL: https://github.com/apache/dubbo/pull/11223#discussion_r1066507383


##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java:
##########
@@ -829,6 +837,18 @@ private void registerServiceInstance() {
                 if (applicationModel.isDestroyed()) {
                     return;
                 }
+
+                // refresh for 30 times (default for 30s) when deployer is not started, prevent submit too many revision

Review Comment:
   Should this be the following according to the logic?
   
   // refresh after waiting for 30s before deployer is started to prevent submitting too many revision; 
   // refresh if deployer has already been started.



##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/deploy/DefaultApplicationDeployer.java:
##########
@@ -829,6 +837,18 @@ private void registerServiceInstance() {
                 if (applicationModel.isDestroyed()) {
                     return;
                 }
+
+                // refresh for 30 times (default for 30s) when deployer is not started, prevent submit too many revision
+                if (instanceRefreshScheduleTimes.incrementAndGet() % 30 != 0 && !isStarted()) {
+                    return;
+                }
+
+                // refresh for 5 times (default for 5s) when services are being updated by other threads, prevent submit too many revision
+                // note: should not always wait here

Review Comment:
   // Wait for 5s if services are in the middle of an updating process (identifies by the status of `serviceRefreshState`) to avoid submitting too many ephemeral revisions



##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryProtocol.java:
##########
@@ -210,7 +211,13 @@ public Map<URL, Set<NotifyListener>> getOverrideListeners() {
     }
 
     private void register(Registry registry, URL registeredProviderUrl) {
-        registry.register(registeredProviderUrl);
+        ApplicationDeployer deployer = registeredProviderUrl.getOrDefaultApplicationModel().getDeployer();
+        try {
+            deployer.increaseServiceRefreshCount();
+            registry.register(registeredProviderUrl);
+        } finally {
+            deployer.decreaseServiceRefreshCount();

Review Comment:
   The refresh starting state should be marked at the beginning of a batch service update, and being marked as finished when batch updating finished. Currently, the state only is the state of one service, so ephemeral revisions might still happen.  



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