From 7803e335749fcdc929f56af6a80f1772ee09e984 Mon Sep 17 00:00:00 2001 From: zrlw Date: Fri, 24 Nov 2023 16:25:24 +0800 Subject: [PATCH] get retained instances only based on ip and port checking (#11342) * only compare ip and port between deregister instance and redo instance * add unit test for getRetainInstance * fix unit test for checkstyle check * format codes only to trigger Codecov checking --- .../remote/gprc/NamingGrpcClientProxy.java | 42 +++++++++++++------ .../gprc/NamingGrpcClientProxyTest.java | 30 +++++++++++++ 2 files changed, 60 insertions(+), 12 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 index 27c756b3a..95ade8be1 100644 --- 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 @@ -62,6 +62,7 @@ import com.alibaba.nacos.common.utils.JacksonUtils; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -153,43 +154,60 @@ public class NamingGrpcClientProxy extends AbstractNamingClientProxy { * * @param serviceName service name * @param groupName group name - * @param instances instance list - * @return instance list that need to be deregistered. + * @param deRegisterInstances deregister instance list + * @return instance list that need to be retained. */ - private List getRetainInstance(String serviceName, String groupName, List instances) + private List getRetainInstance(String serviceName, String groupName, List deRegisterInstances) throws NacosException { - if (CollectionUtils.isEmpty(instances)) { + if (CollectionUtils.isEmpty(deRegisterInstances)) { throw new NacosException(NacosException.INVALID_PARAM, String.format("[Batch deRegistration] need deRegister instance is empty, instances: %s,", - instances)); + deRegisterInstances)); } String combinedServiceName = NamingUtils.getGroupedName(serviceName, groupName); InstanceRedoData instanceRedoData = redoService.getRegisteredInstancesByKey(combinedServiceName); if (!(instanceRedoData instanceof BatchInstanceRedoData)) { throw new NacosException(NacosException.INVALID_PARAM, String.format( "[Batch deRegistration] batch deRegister is not BatchInstanceRedoData type , instances: %s,", - instances)); + deRegisterInstances)); } BatchInstanceRedoData batchInstanceRedoData = (BatchInstanceRedoData) instanceRedoData; - List allInstance = batchInstanceRedoData.getInstances(); - if (CollectionUtils.isEmpty(allInstance)) { + List allRedoInstances = batchInstanceRedoData.getInstances(); + if (CollectionUtils.isEmpty(allRedoInstances)) { throw new NacosException(NacosException.INVALID_PARAM, String.format( "[Batch deRegistration] not found all registerInstance , serviceNameļ¼š%s , groupName: %s", serviceName, groupName)); } - Map instanceMap = instances.stream() + Map deRegisterInstanceMap = deRegisterInstances.stream() .collect(Collectors.toMap(Function.identity(), Function.identity())); List retainInstances = new ArrayList<>(); - for (Instance instance : allInstance) { - if (!instanceMap.containsKey(instance)) { - retainInstances.add(instance); + for (Instance redoInstance : allRedoInstances) { + boolean needRetained = true; + Iterator> it = deRegisterInstanceMap.entrySet().iterator(); + while (it.hasNext()) { + Instance deRegisterInstance = it.next().getKey(); + // only compare Ip & Port because redoInstance's instanceId or serviceName might be null but deRegisterInstance's might not be null. + if (compareIpAndPort(deRegisterInstance, redoInstance)) { + needRetained = false; + // clear current entry to speed up next redoInstance comparing. + it.remove(); + break; + } + } + if (needRetained) { + retainInstances.add(redoInstance); } } return retainInstances; } + private boolean compareIpAndPort(Instance deRegisterInstance, Instance redoInstance) { + return ((deRegisterInstance.getIp().equals(redoInstance.getIp())) + && (deRegisterInstance.getPort() == redoInstance.getPort())); + } + /** * Execute batch register operation. * diff --git a/client/src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java b/client/src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java index d234e37dd..cd176d098 100644 --- a/client/src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java +++ b/client/src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java @@ -327,6 +327,36 @@ public class NamingGrpcClientProxyTest { })); } + @Test + public void testBatchDeregisterServiceWithOtherPortInstance() throws NacosException { + try { + List 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); + Instance otherPortInstance = new Instance(); + otherPortInstance.setServiceName(SERVICE_NAME); + otherPortInstance.setIp("1.1.1.1"); + otherPortInstance.setPort(2222); + List instanceList = new ArrayList<>(); + instanceList.add(otherPortInstance); + client.batchDeregisterService(SERVICE_NAME, GROUP_NAME, instanceList); + verify(this.rpcClient, times(2)).request(argThat(request -> { + if (request instanceof BatchInstanceRequest) { + BatchInstanceRequest request1 = (BatchInstanceRequest) request; + request1.setRequestId("1"); + return request1.getInstances().size() == 2 && request1.getType() + .equals(NamingRemoteConstants.BATCH_REGISTER_INSTANCE); + } + return false; + })); + } + @Test public void testUpdateInstance() throws Exception { //TODO thrown.expect(UnsupportedOperationException.class);