You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2022/07/15 10:36:31 UTC

[GitHub] [dubbo] 24kpure commented on a diff in pull request #10323: Multiple registries

24kpure commented on code in PR #10323:
URL: https://github.com/apache/dubbo/pull/10323#discussion_r922044419


##########
dubbo-registry/dubbo-registry-multiple/src/main/java/org/apache/dubbo/registry/multiple/MultipleRegistry.java:
##########
@@ -292,15 +302,49 @@ public synchronized void notifySourceListener() {
                     }
                     continue;
                 }
-                notifyURLs.addAll(tmpUrls);
+                URL registryURL = singleNotifyListener.getRegistry().getUrl();
+                aggregateRegistryUrls(notifyURLs, tmpUrls, registryURL);
             }
             // if no notify URL, add empty protocol URL
             if (emptyURL != null && notifyURLs.isEmpty()) {
                 notifyURLs.add(emptyURL);
             }
+
+            LOGGER.info("Aggregated url size " + notifyURLs.size());
             this.notify(notifyURLs);
         }
 
+        /**
+         * Aggregate urls from different registries into one unified list while appending registry specific 'attachments' into each url.
+         *
+         * These 'attachments' can be very useful for traffic management among registries.
+         *
+         * @param notifyURLs unified url list
+         * @param singleURLs single registry url list
+         * @param registryURL single registry configuration url
+         */
+        private void aggregateRegistryUrls(List<URL> notifyURLs, List<URL> singleURLs, URL registryURL) {
+            String registryAttachments = registryURL.getParameter("attachments");
+            if (StringUtils.isNotBlank(registryAttachments)) {
+                LOGGER.info("Registry attachments " + registryAttachments + " found, will append to provider urls, urls size " + singleURLs.size());
+                String[] pairs = registryAttachments.split(COMMA_SEPARATOR);
+                for (String rawPair : pairs) {
+                   String[] keyValuePair = rawPair.split("=");

Review Comment:
   Here are my personal opinion,just some typo.
   1.I find `split("=")` appear twice already in project.How about EQUAL_SEPARATOR?
   2.Could we  remove `else` with the code like  below
    `if (StringUtils.isBlank(registryAttachments)) {
              //do somthing
               return;
           }`
   3. Do some `for` in this method can be replace with stream?



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