Skip to content

Commit

Permalink
get retained instances only based on ip and port checking (#11342)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
zrlw authored Nov 24, 2023
1 parent 44b0891 commit 7803e33
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -153,43 +154,60 @@ public void batchDeregisterService(String serviceName, String groupName, List<In
*
* @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<Instance> getRetainInstance(String serviceName, String groupName, List<Instance> instances)
private List<Instance> getRetainInstance(String serviceName, String groupName, List<Instance> 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<Instance> allInstance = batchInstanceRedoData.getInstances();
if (CollectionUtils.isEmpty(allInstance)) {
List<Instance> 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<Instance, Instance> instanceMap = instances.stream()
Map<Instance, Instance> deRegisterInstanceMap = deRegisterInstances.stream()
.collect(Collectors.toMap(Function.identity(), Function.identity()));
List<Instance> retainInstances = new ArrayList<>();
for (Instance instance : allInstance) {
if (!instanceMap.containsKey(instance)) {
retainInstances.add(instance);
for (Instance redoInstance : allRedoInstances) {
boolean needRetained = true;
Iterator<Map.Entry<Instance, Instance>> 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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,36 @@ public void testBatchDeregisterService() throws NacosException {
}));
}

@Test
public void testBatchDeregisterServiceWithOtherPortInstance() 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);
Instance otherPortInstance = new Instance();
otherPortInstance.setServiceName(SERVICE_NAME);
otherPortInstance.setIp("1.1.1.1");
otherPortInstance.setPort(2222);
List<Instance> 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);
Expand Down

0 comments on commit 7803e33

Please sign in to comment.