This is an automated email from the ASF dual-hosted git repository. liujun pushed a commit to branch 3.0 in repository https://gitbox.apache.org/repos/asf/dubbo.git
commit cbca885fa8d1bf16e1e0d95ff6e89eafa274a69d Author: ken.lj <[email protected]> AuthorDate: Wed Sep 9 14:59:48 2020 +0800 fix service discovery address notification and migration bugs --- .../client/ServiceDiscoveryRegistryDirectory.java | 3 +++ .../listener/ServiceInstancesChangedListener.java | 30 +++++++++++----------- .../DefaultMigrationAddressComparator.java | 2 +- .../client/migration/MigrationInvoker.java | 9 ++++--- .../client/migration/MigrationRuleHandler.java | 24 +++++++++-------- .../java/org/apache/dubbo/registry/ZKTools.java | 2 +- 6 files changed, 40 insertions(+), 30 deletions(-) diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistryDirectory.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistryDirectory.java index 306350a..df573d1 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistryDirectory.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistryDirectory.java @@ -127,6 +127,9 @@ public class ServiceDiscoveryRegistryDirectory<T> extends DynamicDirectory<T> im logger.warn("destroyUnusedInvokers error. ", e); } } + + // notify invokers refreshed + this.invokersChanged(); } /** diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java index 7a833e4..c412547 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/event/listener/ServiceInstancesChangedListener.java @@ -108,7 +108,7 @@ public class ServiceInstancesChangedListener implements ConditionalEventListener metadata = getMetadataInfo(instance); logger.info("MetadataInfo for instance " + instance.getAddress() + "?revision=" + revision + " is " + metadata); if (metadata != null) { - revisionToMetadata.put(revision, getMetadataInfo(instance)); + revisionToMetadata.put(revision, metadata); } else { } @@ -123,23 +123,23 @@ public class ServiceInstancesChangedListener implements ConditionalEventListener // Set<String> set = localServiceToRevisions.computeIfAbsent(url.getServiceKey(), k -> new TreeSet<>()); // set.add(revision); // } + } - localServiceToRevisions.forEach((serviceKey, revisions) -> { - List<URL> urls = revisionsToUrls.get(revisions); - if (urls != null) { - serviceUrls.put(serviceKey, urls); - } else { - urls = new ArrayList<>(); - for (String r : revisions) { - for (ServiceInstance i : revisionToInstances.get(r)) { - urls.add(i.toURL()); - } + localServiceToRevisions.forEach((serviceKey, revisions) -> { + List<URL> urls = revisionsToUrls.get(revisions); + if (urls != null) { + serviceUrls.put(serviceKey, urls); + } else { + urls = new ArrayList<>(); + for (String r : revisions) { + for (ServiceInstance i : revisionToInstances.get(r)) { + urls.add(i.toURL()); } - revisionsToUrls.put(revisions, urls); - serviceUrls.put(serviceKey, urls); } - }); - } + revisionsToUrls.put(revisions, urls); + serviceUrls.put(serviceKey, urls); + } + }); } this.notifyAddressChanged(); diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/DefaultMigrationAddressComparator.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/DefaultMigrationAddressComparator.java index eace753..b13ac27 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/DefaultMigrationAddressComparator.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/DefaultMigrationAddressComparator.java @@ -62,7 +62,7 @@ public class DefaultMigrationAddressComparator implements MigrationAddressCompar return false; } - if ((float) (newAddressSize / oldAddressSize) >= threshold) { + if (((float)newAddressSize / (float)oldAddressSize) >= threshold) { return true; } return false; diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationInvoker.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationInvoker.java index c6ba259..e82d164 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationInvoker.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationInvoker.java @@ -141,11 +141,11 @@ public class MigrationInvoker<T> implements MigrationClusterInvoker<T> { @Override public Result invoke(Invocation invocation) throws RpcException { - if (needRefresh(serviceDiscoveryInvoker)) { + if (!checkInvokerAvailable(serviceDiscoveryInvoker)) { return invoker.invoke(invocation); } - if (needRefresh(invoker)) { + if (!checkInvokerAvailable(invoker)) { return serviceDiscoveryInvoker.invoke(invocation); } @@ -270,7 +270,10 @@ public class MigrationInvoker<T> implements MigrationClusterInvoker<T> { } private boolean needRefresh(ClusterInvoker<T> invoker) { - return invoker == null || invoker.isDestroyed() || !invoker.isAvailable(); + return invoker == null || invoker.isDestroyed(); } + public boolean checkInvokerAvailable(ClusterInvoker<T> invoker) { + return invoker != null && !invoker.isDestroyed() && invoker.isAvailable(); + } } diff --git a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationRuleHandler.java b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationRuleHandler.java index 556c701..3df53dc 100644 --- a/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationRuleHandler.java +++ b/dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/migration/MigrationRuleHandler.java @@ -32,6 +32,7 @@ public class MigrationRuleHandler<T> { private static final String DUBBO_SERVICEDISCOVERY_MIGRATION = "dubbo.application.service-discovery.migration"; private MigrationInvoker<T> migrationInvoker; + private MigrationStep currentStep; public MigrationRuleHandler(MigrationInvoker<T> invoker) { this.migrationInvoker = invoker; @@ -51,16 +52,19 @@ public class MigrationRuleHandler<T> { step = rule.getStep(); } - switch (step) { - case APPLICATION_FIRST: - migrationInvoker.migrateToServiceDiscoveryInvoker(false); - break; - case FORCE_APPLICATION: - migrationInvoker.migrateToServiceDiscoveryInvoker(true); - break; - case INTERFACE_FIRST: - default: - migrationInvoker.fallbackToInterfaceInvoker(); + if (currentStep == null || currentStep != step) { + currentStep = step; + switch (step) { + case APPLICATION_FIRST: + migrationInvoker.migrateToServiceDiscoveryInvoker(false); + break; + case FORCE_APPLICATION: + migrationInvoker.migrateToServiceDiscoveryInvoker(true); + break; + case INTERFACE_FIRST: + default: + migrationInvoker.fallbackToInterfaceInvoker(); + } } } } diff --git a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/ZKTools.java b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/ZKTools.java index ac24765..86ff3a2 100644 --- a/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/ZKTools.java +++ b/dubbo-registry/dubbo-registry-api/src/test/java/org/apache/dubbo/registry/ZKTools.java @@ -71,7 +71,7 @@ public class ZKTools { public static void testMigrationRule() { String serviceStr = "---\n" + "key: demo-consumer\n" + - "step: APPLICATION_FIRST\n" + + "step: INTERFACE_FIRST\n" + "..."; try { String servicePath = "/dubbo/config/DUBBO_SERVICEDISCOVERY_MIGRATION/demo-consumer.migration";
