Fix service instance update failed (#11236)

apache · Jan 5, 2023 · ad79c9e · ad79c9e
1 parent 2492881
commit ad79c9e
Show file tree

Hide file tree

Showing 10 changed files with 114 additions and 102 deletions.
diff --git a/...fig/dubbo-config-api/src/test/java/org/apache/dubbo/config/mock/MockServiceDiscovery.java b/...fig/dubbo-config-api/src/test/java/org/apache/dubbo/config/mock/MockServiceDiscovery.java
@@ -16,16 +16,16 @@
  */
 package org.apache.dubbo.config.mock;
 
-import org.apache.dubbo.common.URL;
-import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
-import org.apache.dubbo.registry.client.ServiceInstance;
-import org.apache.dubbo.rpc.model.ApplicationModel;
-
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
+import org.apache.dubbo.registry.client.ServiceInstance;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+
 public class MockServiceDiscovery extends AbstractServiceDiscovery {
     private URL registryURL;
 
@@ -48,8 +48,8 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
     }
 
     @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
-        this.serviceInstance = serviceInstance;
+    protected void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
+        this.serviceInstance = newServiceInstance;
     }
 
     @Override

diff --git a/...ig-spring/src/test/java/org/apache/dubbo/config/spring/registry/MockServiceDiscovery.java b/...ig-spring/src/test/java/org/apache/dubbo/config/spring/registry/MockServiceDiscovery.java
@@ -16,16 +16,16 @@
  */
 package org.apache.dubbo.config.spring.registry;
 
-import org.apache.dubbo.common.URL;
-import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
-import org.apache.dubbo.registry.client.ServiceInstance;
-import org.apache.dubbo.rpc.model.ApplicationModel;
-
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
+import org.apache.dubbo.registry.client.ServiceInstance;
+import org.apache.dubbo.rpc.model.ApplicationModel;
+
 public class MockServiceDiscovery extends AbstractServiceDiscovery {
     private URL registryURL;
 
@@ -47,7 +47,7 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
     }
 
     @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
+    public void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
     }
 
     @Override

diff --git a/...rnetes/src/main/java/org/apache/dubbo/registry/kubernetes/KubernetesServiceDiscovery.java b/...rnetes/src/main/java/org/apache/dubbo/registry/kubernetes/KubernetesServiceDiscovery.java
@@ -16,6 +16,15 @@
  */
 package org.apache.dubbo.registry.kubernetes;
 
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
 import org.apache.dubbo.common.logger.LoggerFactory;
@@ -31,6 +40,7 @@
 import org.apache.dubbo.rpc.model.ScopeModelUtil;
 
 import com.alibaba.fastjson.JSONObject;
+
 import io.fabric8.kubernetes.api.model.EndpointAddress;
 import io.fabric8.kubernetes.api.model.EndpointPort;
 import io.fabric8.kubernetes.api.model.EndpointSubset;
@@ -44,18 +54,9 @@
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
 
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.stream.Collectors;
-
+import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_UNABLE_ACCESS_KUBERNETES;
 import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_UNABLE_FIND_SERVICE_KUBERNETES;
 import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_UNABLE_MATCH_KUBERNETES;
-import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_UNABLE_ACCESS_KUBERNETES;
 
 public class KubernetesServiceDiscovery extends AbstractServiceDiscovery {
     private final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(getClass());
@@ -141,12 +142,12 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
     }
 
     /**
-     * Comparing to {@link AbstractServiceDiscovery#doUpdate(ServiceInstance)}, unregister() is unnecessary here.
+     * Comparing to {@link AbstractServiceDiscovery#doUpdate(ServiceInstance, ServiceInstance)}, unregister() is unnecessary here.
      */
     @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
-        reportMetadata(serviceInstance.getServiceMetadata());
-        this.doRegister(serviceInstance);
+    public void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
+        reportMetadata(newServiceInstance.getServiceMetadata());
+        this.doRegister(newServiceInstance);
     }
 
     @Override

diff --git a/...es/src/test/java/org/apache/dubbo/registry/kubernetes/KubernetesServiceDiscoveryTest.java b/...es/src/test/java/org/apache/dubbo/registry/kubernetes/KubernetesServiceDiscoveryTest.java
@@ -16,6 +16,10 @@
  */
 package org.apache.dubbo.registry.kubernetes;
 
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.config.ApplicationConfig;
 import org.apache.dubbo.registry.client.DefaultServiceInstance;
@@ -25,6 +29,14 @@
 import org.apache.dubbo.registry.kubernetes.util.KubernetesClientConst;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 import org.apache.dubbo.rpc.model.ScopeModelUtil;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import io.fabric8.kubernetes.api.model.Endpoints;
 import io.fabric8.kubernetes.api.model.EndpointsBuilder;
@@ -35,18 +47,6 @@
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.NamespacedKubernetesClient;
 import io.fabric8.kubernetes.client.server.mock.KubernetesServer;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mockito;
-import org.mockito.junit.jupiter.MockitoExtension;
-
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
 
 import static org.apache.dubbo.registry.kubernetes.util.KubernetesClientConst.NAMESPACE;
 import static org.awaitility.Awaitility.await;
@@ -175,7 +175,7 @@ void testPodsUpdate() throws Exception {
         serviceDiscovery.addServiceInstancesChangedListener(mockListener);
 
         serviceInstance = new DefaultServiceInstance(SERVICE_NAME, "Test12345", 12345, ScopeModelUtil.getApplicationModel(serviceDiscovery.getUrl().getScopeModel()));
-        serviceDiscovery.doUpdate(serviceInstance);
+        serviceDiscovery.doUpdate(serviceInstance, serviceInstance);
 
         await().until(() -> {
             ArgumentCaptor<ServiceInstancesChangedEvent> captor = ArgumentCaptor.forClass(ServiceInstancesChangedEvent.class);
@@ -236,7 +236,7 @@ void testGetInstance() {
 
         serviceDiscovery.doRegister(serviceInstance);
 
-        serviceDiscovery.doUpdate(serviceInstance);
+        serviceDiscovery.doUpdate(serviceInstance, serviceInstance);
 
         Assertions.assertEquals(1, serviceDiscovery.getServices().size());
         Assertions.assertEquals(1, serviceDiscovery.getInstances(SERVICE_NAME).size());

diff --git a/...registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java b/...registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java
@@ -16,6 +16,10 @@
  */
 package org.apache.dubbo.registry.client;
 
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
 import org.apache.dubbo.common.logger.LoggerFactory;
@@ -33,10 +37,6 @@
 import org.apache.dubbo.registry.client.metadata.store.MetaCacheManager;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
-
 import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_METADATA_STORAGE_TYPE;
 import static org.apache.dubbo.common.constants.CommonConstants.REGISTRY_LOCAL_FILE_CACHE_ENABLED;
 import static org.apache.dubbo.common.constants.CommonConstants.REMOTE_METADATA_STORAGE_TYPE;
@@ -130,10 +130,13 @@ public synchronized void update() throws RuntimeException {
             return;
         }
 
-        boolean revisionUpdated = calOrUpdateInstanceRevision(this.serviceInstance);
+        ServiceInstance oldServiceInstance = this.serviceInstance;
+        DefaultServiceInstance newServiceInstance = new DefaultServiceInstance((DefaultServiceInstance) oldServiceInstance);
+        boolean revisionUpdated = calOrUpdateInstanceRevision(newServiceInstance);
         if (revisionUpdated) {
-            logger.info(String.format("Metadata of instance changed, updating instance with revision %s.", this.serviceInstance.getServiceMetadata().getRevision()));
-            doUpdate(this.serviceInstance);
+            logger.info(String.format("Metadata of instance changed, updating instance with revision %s.", newServiceInstance.getServiceMetadata().getRevision()));
+            doUpdate(oldServiceInstance, newServiceInstance);
+            this.serviceInstance = newServiceInstance;
         }
     }
 
@@ -247,12 +250,13 @@ public List<URL> lookup(URL url) {
         throw new UnsupportedOperationException("Service discovery implementation does not support lookup of url list.");
     }
 
-    protected void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
-        this.unregister();
+    protected void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
+        this.doUnregister(oldServiceInstance);
 
         if (!EMPTY_REVISION.equals(getExportedServicesRevision(serviceInstance))) {
             reportMetadata(serviceInstance.getServiceMetadata());
-            this.doRegister(serviceInstance);
+            this.serviceInstance = newServiceInstance;
+            this.doRegister(newServiceInstance);
         }
     }
 

diff --git a/...y-api/src/main/java/org/apache/dubbo/registry/client/ReflectionBasedServiceDiscovery.java b/...y-api/src/main/java/org/apache/dubbo/registry/client/ReflectionBasedServiceDiscovery.java
@@ -16,6 +16,19 @@
  */
 package org.apache.dubbo.registry.client;
 
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.constants.CommonConstants;
 import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
@@ -39,19 +52,6 @@
 
 import com.alibaba.fastjson.JSONObject;
 
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-
 import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_FAILED_NOTIFY_EVENT;
 
 public class ReflectionBasedServiceDiscovery extends AbstractServiceDiscovery {
@@ -180,11 +180,6 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
         updateInstanceMetadata(serviceInstance);
     }
 
-    @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
-        updateInstanceMetadata(serviceInstance);
-    }
-
     @Override
     public void doUnregister(ServiceInstance serviceInstance) throws RuntimeException {
         // notify empty message to consumer

diff --git a/...stry-api/src/test/java/org/apache/dubbo/registry/client/support/MockServiceDiscovery.java b/...stry-api/src/test/java/org/apache/dubbo/registry/client/support/MockServiceDiscovery.java
@@ -16,16 +16,16 @@
  */
 package org.apache.dubbo.registry.client.support;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
 import org.apache.dubbo.registry.client.ServiceInstance;
 import org.apache.dubbo.registry.client.metadata.store.MetaCacheManager;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
-
 public class MockServiceDiscovery extends AbstractServiceDiscovery {
     public MockServiceDiscovery(ApplicationModel applicationModel, URL registryURL) {
         super(applicationModel, registryURL);
@@ -41,7 +41,7 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
     }
 
     @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
+    public void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
 
     }
 

diff --git a/...ulticast/src/main/java/org/apache/dubbo/registry/multicast/MulticastServiceDiscovery.java b/...ulticast/src/main/java/org/apache/dubbo/registry/multicast/MulticastServiceDiscovery.java
@@ -16,15 +16,15 @@
  */
 package org.apache.dubbo.registry.multicast;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.registry.client.AbstractServiceDiscovery;
 import org.apache.dubbo.registry.client.ServiceInstance;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
-
 /**
  * TODO: make multicast protocol support Service Discovery
  */
@@ -47,7 +47,7 @@ public void doRegister(ServiceInstance serviceInstance) throws RuntimeException
     }
 
     @Override
-    public void doUpdate(ServiceInstance serviceInstance) throws RuntimeException {
+    public void doUpdate(ServiceInstance oldServiceInstance, ServiceInstance newServiceInstance) throws RuntimeException {
     }
 
     @Override

diff --git a/...o-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java b/...o-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosServiceDiscovery.java
@@ -16,6 +16,12 @@
  */
 package org.apache.dubbo.registry.nacos;
 
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
 import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.config.ConfigurationUtils;
 import org.apache.dubbo.common.function.ThrowableFunction;
@@ -37,12 +43,6 @@
 import com.alibaba.nacos.api.naming.pojo.Instance;
 import com.alibaba.nacos.api.naming.pojo.ListView;
 
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Collectors;
-
 import static com.alibaba.nacos.api.common.Constants.DEFAULT_GROUP;
 import static org.apache.dubbo.common.constants.LoggerCodeConstants.REGISTRY_NACOS_EXCEPTION;
 import static org.apache.dubbo.common.function.ThrowableConsumer.execute;