Skip to content

Commit

Permalink
[ISSUE #9816] Add expected final state for redo data. (#9893)
Browse files Browse the repository at this point in the history 
* Fix #9816, add expected final state to make redo task execute right finally.

* Upgrade version to 2.2.1-SNAPSHOT.
  • Loading branch information
@KomachiSion
KomachiSion committed Feb 3, 2023
1 parent 71389b0 commit 4db3e8c
Show file tree
Hide file tree
Showing 9 changed files with 251 additions and 31 deletions.
4 changes: 2 additions & 2 deletions client/src/main/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxy.java
View file
Expand Up @@ -153,7 +153,7 @@ private List<Instance> getRetainInstance(String serviceName, String groupName, L
instances));
}
String combinedServiceName = NamingUtils.getGroupedName(serviceName, groupName);
InstanceRedoData instanceRedoData = redoService.getRegisteredInstancesBykey(combinedServiceName);
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,",
Expand Down Expand Up @@ -231,7 +231,7 @@ public void doDeregisterService(String serviceName, String groupName, Instance i
InstanceRequest request = new InstanceRequest(namespaceId, serviceName, groupName,
NamingRemoteConstants.DE_REGISTER_INSTANCE, instance);
requestToServer(request, Response.class);
redoService.removeInstanceForRedo(serviceName, groupName);
redoService.instanceDeregistered(serviceName, groupName);
}

@Override
Expand Down
21 changes: 19 additions & 2 deletions ...src/main/java/com/alibaba/nacos/client/naming/remote/gprc/redo/NamingGrpcRedoService.java
View file
Expand Up @@ -132,7 +132,7 @@ public void instanceRegistered(String serviceName, String groupName) {
synchronized (registeredInstances) {
InstanceRedoData redoData = registeredInstances.get(key);
if (null != redoData) {
redoData.setRegistered(true);
redoData.registered();
}
}
}
Expand All @@ -149,6 +149,23 @@ public void instanceDeregister(String serviceName, String groupName) {
InstanceRedoData redoData = registeredInstances.get(key);
if (null != redoData) {
redoData.setUnregistering(true);
redoData.setExpectedRegistered(false);
}
}
}

/**
* Instance deregister finished, mark unregistered status.
*
* @param serviceName service name
* @param groupName group name
*/
public void instanceDeregistered(String serviceName, String groupName) {
String key = NamingUtils.getGroupedName(serviceName, groupName);
synchronized (registeredInstances) {
InstanceRedoData redoData = registeredInstances.get(key);
if (null != redoData) {
redoData.unregistered();
}
}
}
Expand Down Expand Up @@ -281,7 +298,7 @@ public Set<SubscriberRedoData> findSubscriberRedoData() {
* get Cache service.
* @return cache service
*/
public InstanceRedoData getRegisteredInstancesBykey(String combinedServiceName) {
public InstanceRedoData getRegisteredInstancesByKey(String combinedServiceName) {
return registeredInstances.get(combinedServiceName);
}

Expand Down
21 changes: 21 additions & 0 deletions ...ain/java/com/alibaba/nacos/client/naming/remote/gprc/redo/data/BatchInstanceRedoData.java
View file
Expand Up @@ -19,6 +19,7 @@
import com.alibaba.nacos.api.naming.pojo.Instance;

import java.util.List;
import java.util.Objects;

/**
* batch instance redo service.
Expand Down Expand Up @@ -54,4 +55,24 @@ public static BatchInstanceRedoData build(String serviceName, String groupName,
result.setInstances(instances);
return result;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof BatchInstanceRedoData)) {
return false;
}
if (!super.equals(o)) {
return false;
}
BatchInstanceRedoData redoData = (BatchInstanceRedoData) o;
return Objects.equals(instances, redoData.instances);
}

@Override
public int hashCode() {
return Objects.hash(super.hashCode(), instances);
}
}
60 changes: 45 additions & 15 deletions client/src/main/java/com/alibaba/nacos/client/naming/remote/gprc/redo/data/RedoData.java
View file
Expand Up @@ -30,6 +30,16 @@ public abstract class RedoData<T> {

private final String groupName;

/**
* Expected states for finally.
*
* <ul>
* <li>{@code true} meas the cached data expect registered to server finally.</li>
* <li>{@code false} means unregistered from server.</li>
* </ul>
*/
private volatile boolean expectedRegistered;

/**
* If {@code true} means cached data has been registered to server successfully.
*/
Expand All @@ -45,6 +55,7 @@ public abstract class RedoData<T> {
protected RedoData(String serviceName, String groupName) {
this.serviceName = serviceName;
this.groupName = groupName;
this.expectedRegistered = true;
}

public String getServiceName() {
Expand All @@ -55,18 +66,26 @@ public String getGroupName() {
return groupName;
}

public boolean isRegistered() {
return registered;
public void setExpectedRegistered(boolean registered) {
this.expectedRegistered = registered;
}

public void setRegistered(boolean registered) {
this.registered = registered;
public boolean isExpectedRegistered() {
return expectedRegistered;
}

public boolean isRegistered() {
return registered;
}

public boolean isUnregistering() {
return unregistering;
}

public void setRegistered(boolean registered) {
this.registered = registered;
}

public void setUnregistering(boolean unregistering) {
this.unregistering = unregistering;
}
Expand All @@ -79,8 +98,22 @@ public void set(T data) {
this.data = data;
}

public void registered() {
this.registered = true;
this.unregistering = false;
}

public void unregistered() {
this.registered = false;
this.unregistering = true;
}

public boolean isNeedRedo() {
return !RedoType.NONE.equals(getRedoType());
}

/**
* Get redo type for current redo data.
* Get redo type for current redo data without expected state.
*
* <ul>
* <li>{@code registered=true} & {@code unregistering=false} means data has registered, so redo should not do anything.</li>
Expand All @@ -93,20 +126,16 @@ public void set(T data) {
*/
public RedoType getRedoType() {
if (isRegistered() && !isUnregistering()) {
return RedoType.NONE;
return expectedRegistered ? RedoType.NONE : RedoType.UNREGISTER;
} else if (isRegistered() && isUnregistering()) {
return RedoType.UNREGISTER;
} else if (!isRegistered() && !isUnregistering()) {
return RedoType.REGISTER;
} else {
return RedoType.REMOVE;
return expectedRegistered ? RedoType.REGISTER : RedoType.REMOVE;
}
}

public boolean isNeedRedo() {
return !RedoType.NONE.equals(getRedoType());
}

public enum RedoType {

/**
Expand All @@ -129,7 +158,7 @@ public enum RedoType {
*/
REMOVE;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -139,10 +168,11 @@ public boolean equals(Object o) {
return false;
}
RedoData<?> redoData = (RedoData<?>) o;
return registered == redoData.registered && unregistering == redoData.unregistering
&& serviceName.equals(redoData.serviceName) && groupName.equals(redoData.groupName) && data.equals(redoData.data);
return registered == redoData.registered && unregistering == redoData.unregistering && serviceName
.equals(redoData.serviceName) && groupName.equals(redoData.groupName) && Objects
.equals(data, redoData.data);
}

@Override
public int hashCode() {
return Objects.hash(serviceName, groupName, registered, unregistering, data);
Expand Down
76 changes: 75 additions & 1 deletion .../src/test/java/com/alibaba/nacos/client/naming/remote/gprc/NamingGrpcClientProxyTest.java
View file
Expand Up @@ -66,6 +66,7 @@
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -74,6 +75,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -223,6 +226,66 @@ public void testBatchRegisterService() throws NacosException {
}));
}

@Test(expected = NacosException.class)
public void testBatchDeregisterServiceWithEmptyInstances() throws NacosException {
client.batchDeregisterService(SERVICE_NAME, GROUP_NAME, Collections.EMPTY_LIST);
}

@Test(expected = NacosException.class)
public void testBatchDeregisterServiceWithoutCacheData() throws NacosException {
List<Instance> instanceList = new ArrayList<>();
instance.setHealthy(true);
instanceList.add(instance);
client.batchDeregisterService(SERVICE_NAME, GROUP_NAME, instanceList);
}

@Test(expected = NacosException.class)
public void testBatchDeregisterServiceNotBatchData() throws NacosException {
client.registerService(SERVICE_NAME, GROUP_NAME, instance);
List<Instance> instanceList = new ArrayList<>();
instance.setHealthy(true);
instanceList.add(instance);
client.batchDeregisterService(SERVICE_NAME, GROUP_NAME, instanceList);
}

@Test(expected = NacosException.class)
public void testBatchDeregisterServiceWithEmptyBatchData() throws NacosException {
try {
client.batchRegisterService(SERVICE_NAME, GROUP_NAME, Collections.EMPTY_LIST);
} catch (Exception ignored) {
}
List<Instance> instanceList = new ArrayList<>();
instance.setHealthy(true);
instanceList.add(instance);
client.batchDeregisterService(SERVICE_NAME, GROUP_NAME, instanceList);
}

@Test
public void testBatchDeregisterService() 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.batchDeregisterService(SERVICE_NAME, GROUP_NAME, instanceList);
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 testUpdateInstance() throws Exception {
//TODO thrown.expect(UnsupportedOperationException.class);
Expand Down Expand Up @@ -256,7 +319,7 @@ public void testCreateService() throws Exception {
@Test
public void testDeleteService() throws Exception {
//TODO thrown.expect(UnsupportedOperationException.class);
Assert.assertFalse(client.deleteService(SERVICE_NAME, GROUP_NAME));
assertFalse(client.deleteService(SERVICE_NAME, GROUP_NAME));
}

@Test
Expand Down Expand Up @@ -310,6 +373,17 @@ public void testUnsubscribe() throws Exception {
}));
}

@Test
public void testIsSubscribed() throws NacosException {
SubscribeServiceResponse res = new SubscribeServiceResponse();
ServiceInfo info = new ServiceInfo(GROUP_NAME + "@@" + SERVICE_NAME + "@@" + CLUSTERS);
res.setServiceInfo(info);
when(this.rpcClient.request(any())).thenReturn(res);
assertFalse(client.isSubscribed(SERVICE_NAME, GROUP_NAME, CLUSTERS));
client.subscribe(SERVICE_NAME, GROUP_NAME, CLUSTERS);
assertTrue(client.isSubscribed(SERVICE_NAME, GROUP_NAME, CLUSTERS));
}

@Test
public void testServerHealthy() {
when(this.rpcClient.isRunning()).thenReturn(true);
Expand Down
20 changes: 20 additions & 0 deletions ...test/java/com/alibaba/nacos/client/naming/remote/gprc/redo/NamingGrpcRedoServiceTest.java
View file
Expand Up @@ -101,6 +101,7 @@ public void testCacheInstanceForRedo() {
assertEquals(instance, actual.get());
assertFalse(actual.isRegistered());
assertFalse(actual.isUnregistering());
assertTrue(actual.isExpectedRegistered());
}

@Test
Expand Down Expand Up @@ -136,6 +137,17 @@ public void testInstanceDeregister() {
redoService.instanceDeregister(SERVICE, GROUP);
InstanceRedoData actual = registeredInstances.entrySet().iterator().next().getValue();
assertTrue(actual.isUnregistering());
assertFalse(actual.isExpectedRegistered());
}

@Test
public void testInstanceDeregistered() {
ConcurrentMap<String, InstanceRedoData> registeredInstances = getInstanceRedoDataMap();
redoService.cacheInstanceForRedo(SERVICE, GROUP, new Instance());
redoService.instanceDeregistered(SERVICE, GROUP);
InstanceRedoData actual = registeredInstances.entrySet().iterator().next().getValue();
assertFalse(actual.isRegistered());
assertTrue(actual.isUnregistering());
}

@Test
Expand Down Expand Up @@ -195,6 +207,14 @@ public void testSubscriberDeregister() {
assertTrue(actual.isUnregistering());
}

@Test
public void testIsSubscriberRegistered() {
assertFalse(redoService.isSubscriberRegistered(SERVICE, GROUP, CLUSTER));
redoService.cacheSubscriberForRedo(SERVICE, GROUP, CLUSTER);
redoService.subscriberRegistered(SERVICE, GROUP, CLUSTER);
assertTrue(redoService.isSubscriberRegistered(SERVICE, GROUP, CLUSTER));
}

@Test
public void testRemoveSubscriberForRedo() {
ConcurrentMap<String, SubscriberRedoData> subscribes = getSubscriberRedoDataMap();
Expand Down

0 comments on commit 4db3e8c

Please sign in to comment.