fix(alibaba#10831): When using the deregisterInstance method to remov…

…e one of multiple instances registered by batchRegisterInstance, all instances registered by batchRegisterInstance will be removed.
Bo-Qiu · Jul 26, 2023 · c7603bb · c7603bb
1 parent 8adca68
commit c7603bb
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 8 deletions.
diff --git a/client/src/main/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxy.java b/client/src/main/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxy.java
@@ -54,8 +54,8 @@
 import com.alibaba.nacos.common.remote.ConnectionType;
 import com.alibaba.nacos.common.remote.client.RpcClient;
 import com.alibaba.nacos.common.remote.client.RpcClientFactory;
-import com.alibaba.nacos.common.remote.client.ServerListFactory;
 import com.alibaba.nacos.common.remote.client.RpcClientTlsConfig;
+import com.alibaba.nacos.common.remote.client.ServerListFactory;
 import com.alibaba.nacos.common.utils.CollectionUtils;
 import com.alibaba.nacos.common.utils.JacksonUtils;
 
@@ -139,8 +139,10 @@ public void batchRegisterService(String serviceName, String groupName, List<Inst
     @Override
     public void batchDeregisterService(String serviceName, String groupName, List<Instance> instances)
             throws NacosException {
-        List<Instance> retainInstance = getRetainInstance(serviceName, groupName, instances);
-        batchRegisterService(serviceName, groupName, retainInstance);
+        synchronized (redoService.getRegisteredInstances()) {
+            List<Instance> retainInstance = getRetainInstance(serviceName, groupName, instances);
+            batchRegisterService(serviceName, groupName, retainInstance);
+        }
     }
 
     /**
@@ -218,11 +220,20 @@ public void doRegisterService(String serviceName, String groupName, Instance ins
 
     @Override
     public void deregisterService(String serviceName, String groupName, Instance instance) throws NacosException {
-        NAMING_LOGGER
-                .info("[DEREGISTER-SERVICE] {} deregistering service {} with instance: {}", namespaceId, serviceName,
-                        instance);
-        redoService.instanceDeregister(serviceName, groupName);
-        doDeregisterService(serviceName, groupName, instance);
+        NAMING_LOGGER.info("[DEREGISTER-SERVICE] {} deregistering service {} with instance: {}", namespaceId,
+                serviceName, instance);
+        String key = NamingUtils.getGroupedName(serviceName, groupName);
+        InstanceRedoData instanceRedoData = redoService.getRegisteredInstancesByKey(key);
+        if (instanceRedoData instanceof BatchInstanceRedoData) {
+            List<Instance> instances = new ArrayList<>();
+            if (null != instance) {
+                instances.add(instance);
+            }
+            batchDeregisterService(serviceName, groupName, instances);
+        } else {
+            redoService.instanceDeregister(serviceName, groupName);
+            doDeregisterService(serviceName, groupName, instance);
+        }
     }
 
     /**

diff --git a/...src/main/java/com/alibaba/nacos/client/naming/remote/gprc/redo/NamingGrpcRedoService.java b/...src/main/java/com/alibaba/nacos/client/naming/remote/gprc/redo/NamingGrpcRedoService.java
@@ -68,6 +68,10 @@ public NamingGrpcRedoService(NamingGrpcClientProxy clientProxy) {
                 DEFAULT_REDO_DELAY, TimeUnit.MILLISECONDS);
     }
 
+    public ConcurrentMap<String, InstanceRedoData> getRegisteredInstances() {
+        return registeredInstances;
+    }
+
     public boolean isConnected() {
         return connected;
     }

diff --git a/.../src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java b/.../src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java
@@ -221,6 +221,33 @@ public void testDeregisterService() throws NacosException {
         }));
     }
 
+    @Test
+    public void testDeregisterServiceForBatchRegistered() throws NacosException {
+        try {
+            List<Instance> instanceList = new ArrayList<>();
+            instance.setHealthy(true);
+            instanceList.add(instance);
+            instanceList.add(new Instance());
+            client.batchRegisterService(SERVICE_NAME, GROUP_NAME, instanceList);
+        } catch (Exception ignored) {
+        }
+        response = new BatchInstanceResponse();
+        when(this.rpcClient.request(any())).thenReturn(response);
+        List<Instance> instanceList = new ArrayList<>();
+        instance.setHealthy(true);
+        instanceList.add(instance);
+        client.deregisterService(SERVICE_NAME, GROUP_NAME, instance);
+        verify(this.rpcClient, times(1)).request(argThat(request -> {
+            if (request instanceof BatchInstanceRequest) {
+                BatchInstanceRequest request1 = (BatchInstanceRequest) request;
+                request1.setRequestId("1");
+                return request1.getInstances().size() == 1 && request1.getType()
+                        .equals(NamingRemoteConstants.BATCH_REGISTER_INSTANCE);
+            }
+            return false;
+        }));
+    }
+
     @Test
     public void testBatchRegisterService() throws NacosException {
         List<Instance> instanceList = new ArrayList<>();