[
https://issues.apache.org/jira/browse/SCB-582?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16482231#comment-16482231
]
ASF GitHub Bot commented on SCB-582:
------------------------------------
liubao68 closed pull request #704: [SCB-582]Provide a way to protect instance
removal
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/704
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/demo/demo-pojo/pojo-client/src/main/resources/microservice.yaml
b/demo/demo-pojo/pojo-client/src/main/resources/microservice.yaml
index 7be293d08..47b01f504 100644
--- a/demo/demo-pojo/pojo-client/src/main/resources/microservice.yaml
+++ b/demo/demo-pojo/pojo-client/src/main/resources/microservice.yaml
@@ -23,6 +23,12 @@ servicecomb:
service:
registry:
address: http://127.0.0.1:30100
+ instance:
+ healthCheck:
+ interval: 2
+ watch: false
+ empty:
+ protection: true
handler:
chain:
Consumer:
@@ -32,4 +38,4 @@ servicecomb:
enabled: false
loadbalance:
strategy:
- name: Random
+ name: Random
\ No newline at end of file
diff --git
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceInstance.java
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceInstance.java
index 293ab481f..184aac3ca 100644
---
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceInstance.java
+++
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceInstance.java
@@ -139,6 +139,19 @@ public DataCenterInfo getDataCenterInfo() {
return dataCenterInfo;
}
+ @Override
+ public int hashCode() {
+ return this.instanceId.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof MicroserviceInstance) {
+ return this.instanceId.equals(((MicroserviceInstance) obj).instanceId);
+ }
+ return false;
+ }
+
public void setDataCenterInfo(DataCenterInfo dataCenterInfo) {
this.dataCenterInfo = dataCenterInfo;
}
diff --git
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/config/ServiceRegistryConfig.java
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/config/ServiceRegistryConfig.java
index 113fed0f0..2574acf2a 100644
---
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/config/ServiceRegistryConfig.java
+++
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/config/ServiceRegistryConfig.java
@@ -222,6 +222,14 @@ public int getResendHeartBeatTimes() {
return times < 0 ? DEFAULT_CHECK_TIMES : times;
}
+ public boolean isEmptyInstanceProtectionEnabled() {
+ DynamicBooleanProperty property =
+ DynamicPropertyFactory.getInstance()
+
.getBooleanProperty("servicecomb.service.registry.instance.empty.protection",
+ true);
+ return property.get();
+ }
+
public boolean isPreferIpAddress() {
DynamicBooleanProperty property =
DynamicPropertyFactory.getInstance()
diff --git
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceInstancePing.java
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceInstancePing.java
new file mode 100644
index 000000000..7bbfa8c9a
--- /dev/null
+++
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceInstancePing.java
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.servicecomb.serviceregistry.consumer;
+
+import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+
+/**
+ * SPI interface to ping microservice instance status in Instance Protection
Mode, which means
+ * protection of instance removal and usually used in scenarios where
instances are fixed and changed rarely.
+ */
+public interface MicroserviceInstancePing {
+ int getOrder();
+
+ /**
+ * check if this instance if valid to use
+ * @param instance
+ * @return
+ */
+ boolean ping(MicroserviceInstance instance);
+}
diff --git
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceVersions.java
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceVersions.java
index 0b818df90..14e1a18bd 100644
---
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceVersions.java
+++
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceVersions.java
@@ -23,12 +23,14 @@
import java.util.stream.Collectors;
import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
+import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.api.Const;
import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus;
import
org.apache.servicecomb.serviceregistry.api.response.MicroserviceInstanceChangedEvent;
import
org.apache.servicecomb.serviceregistry.client.http.MicroserviceInstances;
+import org.apache.servicecomb.serviceregistry.config.ServiceRegistryConfig;
import org.apache.servicecomb.serviceregistry.definition.DefinitionConst;
import
org.apache.servicecomb.serviceregistry.task.event.PullMicroserviceVersionsInstancesEvent;
import org.slf4j.Logger;
@@ -144,12 +146,7 @@ private void postPullInstanceEvent(long msTime) {
private void setInstances(List<MicroserviceInstance> pulledInstances, String
rev) {
synchronized (lock) {
- instances = pulledInstances
- .stream()
- .filter(instance -> {
- return MicroserviceInstanceStatus.UP.equals(instance.getStatus());
- })
- .collect(Collectors.toList());
+ instances = mergeInstances(pulledInstances, instances);
for (MicroserviceInstance instance : instances) {
// ensure microserviceVersion exists
versions.computeIfAbsent(instance.getServiceId(), microserviceId -> {
@@ -169,6 +166,28 @@ private void setInstances(List<MicroserviceInstance>
pulledInstances, String rev
}
}
+ private List<MicroserviceInstance> mergeInstances(List<MicroserviceInstance>
pulledInstances,
+ List<MicroserviceInstance> inUseInstances) {
+ List<MicroserviceInstance> upInstances = pulledInstances
+ .stream()
+ .filter(instance -> {
+ return MicroserviceInstanceStatus.UP.equals(instance.getStatus());
+ })
+ .collect(Collectors.toList());
+ if (upInstances.isEmpty() && inUseInstances != null &&
ServiceRegistryConfig.INSTANCE.isEmptyInstanceProtectionEnabled()) {
+ MicroserviceInstancePing ping =
SPIServiceUtils.getPriorityHighestService(MicroserviceInstancePing.class);
+ inUseInstances.stream()
+ .forEach(instance -> {
+ if (!upInstances.contains(instance)) {
+ if (ping.ping(instance)) {
+ upInstances.add(instance);
+ }
+ }
+ });
+ }
+ return upInstances;
+ }
+
public MicroserviceVersionRule getOrCreateMicroserviceVersionRule(String
versionRule) {
// do not use computeIfAbsent
MicroserviceVersionRule microserviceVersionRule =
versionRules.get(versionRule);
diff --git
a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/SimpleMicroserviceInstancePing.java
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/SimpleMicroserviceInstancePing.java
new file mode 100644
index 000000000..9520e08da
--- /dev/null
+++
b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/SimpleMicroserviceInstancePing.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.servicecomb.serviceregistry.consumer;
+
+import java.io.IOException;
+import java.net.Socket;
+
+import org.apache.servicecomb.foundation.common.net.IpPort;
+import org.apache.servicecomb.foundation.common.net.NetUtils;
+import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+
+/**
+ * Simple implementation of .MicroserviceInstancePing using telnet
+ */
+public class SimpleMicroserviceInstancePing implements
MicroserviceInstancePing {
+ @Override
+ public int getOrder() {
+ return 100;
+ }
+
+ @Override
+ public boolean ping(MicroserviceInstance instance) {
+ if (instance.getEndpoints() != null && instance.getEndpoints().size() > 0)
{
+ IpPort ipPort =
NetUtils.parseIpPortFromURI(instance.getEndpoints().get(0));
+ try (Socket s = new Socket(ipPort.getHostOrIp(), ipPort.getPort())) {
+ return true;
+ } catch (IOException e) {
+ // ignore this error
+ }
+ }
+ return false;
+ }
+}
diff --git
a/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.consumer.MicroserviceInstancePing
b/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.consumer.MicroserviceInstancePing
new file mode 100644
index 000000000..ce06621e3
--- /dev/null
+++
b/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.consumer.MicroserviceInstancePing
@@ -0,0 +1,18 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+org.apache.servicecomb.serviceregistry.consumer.SimpleMicroserviceInstancePing
\ No newline at end of file
diff --git
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroServiceInstance.java
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroServiceInstance.java
index 4d4ddc60e..affcf9228 100644
---
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroServiceInstance.java
+++
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroServiceInstance.java
@@ -97,6 +97,16 @@ public void testInitializedValues() {
Assert.assertEquals(MicroserviceInstanceStatus.DOWN,
oMicroserviceInstance.getStatus());
Assert.assertEquals("Test", oMicroserviceInstance.getStage());
Assert.assertEquals("china",
oMicroserviceInstance.getProperties().get("region"));
+
+ Assert.assertEquals(oMicroserviceInstance, oMicroserviceInstance);
+ MicroserviceInstance other = new MicroserviceInstance();
+ other.setInstanceId("testInstanceIDOther");
+ MicroserviceInstance same = new MicroserviceInstance();
+ same.setInstanceId("testInstanceID");
+ Assert.assertNotEquals(oMicroserviceInstance, other);
+ Assert.assertNotEquals(oMicroserviceInstance.hashCode(), other.hashCode());
+ Assert.assertEquals(oMicroserviceInstance, same);
+ Assert.assertEquals(oMicroserviceInstance.hashCode(), same.hashCode());
}
@SuppressWarnings("deprecation")
diff --git
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java
index 24b566f1d..88ba8bfcb 100644
---
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java
+++
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java
@@ -134,6 +134,7 @@ public void findServiceInstance_twoSelectOne() {
mockRegisterMicroservice(appId, microserviceName, "2.0.0");
MicroserviceInstance instance = new MicroserviceInstance();
+ instance.setInstanceId("testid");
instance.setServiceId(v1.getServiceId());
registryClient.registerMicroserviceInstance(instance);
diff --git
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersions.java
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersions.java
index b336d8847..53e1f3842 100644
---
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersions.java
+++
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersions.java
@@ -23,6 +23,7 @@
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.api.Const;
import org.apache.servicecomb.serviceregistry.api.MicroserviceKey;
@@ -77,6 +78,7 @@ public void setUp() throws Exception {
@After
public void tearDown() throws Exception {
+ ArchaiusUtils.resetConfig();
findInstancesResponse = null;
microserviceInstances = null;
}
@@ -148,6 +150,17 @@ public void submitPull() {
microserviceVersions.getVersion(microserviceId).getMicroservice());
}
+ @Test
+ public void submitPullProtection() {
+
ArchaiusUtils.setProperty("servicecomb.service.registry.instance.remove.protection",
true);
+ String microserviceId = "1";
+ setup(microserviceId);
+ microserviceVersions.submitPull();
+ microserviceVersions.submitPull();
+ MicroserviceVersionRule versionRule =
microserviceVersions.getOrCreateMicroserviceVersionRule("0+");
+ Assert.assertSame(versionRule.getInstances().entrySet().size(), 1);
+ }
+
@Test
public void pullInstancesCancel() {
new MockUp<RegistryUtils>() {
diff --git
a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestSimpleMicroserviceInstancePing.java
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestSimpleMicroserviceInstancePing.java
new file mode 100644
index 000000000..f4403d83d
--- /dev/null
+++
b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestSimpleMicroserviceInstancePing.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.serviceregistry.consumer;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.util.ArrayList;
+import java.util.List;
+
+import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestSimpleMicroserviceInstancePing {
+ @Test
+ public void testPing() throws IOException {
+ SimpleMicroserviceInstancePing ping = new SimpleMicroserviceInstancePing();
+ Assert.assertEquals(ping.getOrder(), 100);
+ MicroserviceInstance instance = new MicroserviceInstance();
+ List<String> endpoints = new ArrayList<>();
+ ServerSocket ss = new ServerSocket(35677);
+
+ endpoints.add("http://localhost:35677";);
+ instance.setEndpoints(endpoints);
+ Assert.assertTrue(ping.ping(instance));
+ MicroserviceInstance instance2 = new MicroserviceInstance();
+ Assert.assertFalse(ping.ping(instance2));
+ ss.close();
+ Assert.assertFalse(ping.ping(instance));
+ }
+}
diff --git
a/spring-boot-starter/spring-boot-starter-transport/src/test/java/org/apache/servicecomb/springboot/starter/transport/TestRestServletInitializer.java
b/spring-boot-starter/spring-boot-starter-transport/src/test/java/org/apache/servicecomb/springboot/starter/transport/TestRestServletInitializer.java
index 8952d2be2..bb2664833 100644
---
a/spring-boot-starter/spring-boot-starter-transport/src/test/java/org/apache/servicecomb/springboot/starter/transport/TestRestServletInitializer.java
+++
b/spring-boot-starter/spring-boot-starter-transport/src/test/java/org/apache/servicecomb/springboot/starter/transport/TestRestServletInitializer.java
@@ -51,7 +51,7 @@
private static final String LISTEN_ADDRESS = "127.0.0.1";
- private static final int TEST_PORT = 8080;
+ private static final int TEST_PORT = 8582;
@BeforeClass
public static void beforeClass() {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Provide a way to protection for instance removal
> ------------------------------------------------
>
> Key: SCB-582
> URL: https://issues.apache.org/jira/browse/SCB-582
> Project: Apache ServiceComb
> Issue Type: New Feature
> Components: Java-Chassis
> Reporter: liubao
> Assignee: liubao
> Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
> In some applications, each microservice have fixed instances and rarely
> change. But for some reason, heartbeat to service center is not stable. In
> this scenario, instances will get lost and result in invocation failure.
>
> To deal with this problem, we need to provide a way to protect instance
> removal. That's when there is instance removal, we first check(ping)the
> instance to see if it is reachable. If it's reachable, we don't remove it.
>
> Since this protection is not fit for docker applications or scenarios where
> instances are constantly removed and added, and resources(hosts ,ip, port)
> are reused, this feature is an optional feature and users can open it when
> necessary.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
