You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2019/01/07 12:29:49 UTC
[GitHub] liubao68 closed pull request #1051: [SCB-1083]make some status can
be invoked by custom filters
liubao68 closed pull request #1051: [SCB-1083]make some status can be invoked by custom filters
URL: https://github.com/apache/servicecomb-java-chassis/pull/1051
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-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestBizkeeper.java b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestBizkeeper.java
index 325f59f24..305447e1d 100644
--- a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestBizkeeper.java
+++ b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/TestBizkeeper.java
@@ -60,7 +60,7 @@ private void testTimeout() {
"servicecomb.isolation.Consumer.springmvc.codeFirstBizkeeperTest.testTimeout.timeout.enabled", "true");
ArchaiusUtils.setProperty(
"servicecomb.isolation.Consumer.springmvc.codeFirstBizkeeperTest.testTimeout.timeoutInMilliseconds", 500);
- restTemplate.getForObject(prefix + "/testTimeout?name={1}&delaytime={2}", String.class, "joker", 500);
+ restTemplate.getForObject(prefix + "/testTimeout?name={1}&delaytime={2}", String.class, "joker", 1000);
TestMgr.check("expect: throw timeout exception", "real: not throw timeout exception");
} catch (InvocationException e) {
TestMgr.check(TimeoutException.class, e.getCause().getCause().getClass());
diff --git a/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml b/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml
index 9a4b22032..cf392a807 100644
--- a/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml
+++ b/demo/demo-springmvc/springmvc-client/src/main/resources/microservice.yaml
@@ -61,6 +61,7 @@ servicecomb:
retryEnabled: true
retryOnSame: 1
retryOnNext: 1
+ filter.status.enabled: false
fallbackpolicy:
Consumer:
springmvc:
diff --git a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
index 393dc5257..24fc685d3 100644
--- a/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
+++ b/demo/demo-springmvc/springmvc-server/src/main/resources/microservice.yaml
@@ -26,6 +26,8 @@ service_description:
- path: /test2/testpath
property:
checksession: true
+instance_description:
+ initialStatus : TESTING
servicecomb:
microserviceVersionFactory: org.apache.servicecomb.core.definition.PrivateMicroserviceVersionMetaFactory
service:
diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
index d98d672a3..e49797424 100644
--- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
+++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceHandler2.java
@@ -43,6 +43,7 @@
import org.apache.servicecomb.serviceregistry.ServiceRegistry;
import org.apache.servicecomb.serviceregistry.api.registry.DataCenterInfo;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus;
import org.apache.servicecomb.serviceregistry.cache.InstanceCacheManager;
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTree;
import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTreeNode;
@@ -355,6 +356,150 @@ public void testZoneAwareAndIsolationFilterUsingMockedInvocationWorks() throws E
});
}
+ @Test
+ public void testStatusFilterUsingMockedInvocationWorks() throws Exception {
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.status.enabled", "false");
+
+ Invocation invocation = new NonSwaggerInvocation("testApp", "testMicroserviceName", "0.0.0+", (inv, aysnc) -> {
+ aysnc.success("OK");
+ });
+
+ InstanceCacheManager instanceCacheManager = Mockito.mock(InstanceCacheManager.class);
+ ServiceRegistry serviceRegistry = Mockito.mock(ServiceRegistry.class);
+ TransportManager transportManager = Mockito.mock(TransportManager.class);
+ Transport transport = Mockito.mock(Transport.class);
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.operation.enabled", "false");
+
+ // set up data
+ MicroserviceInstance myself = new MicroserviceInstance();
+ DataCenterInfo info = new DataCenterInfo();
+ info.setName("test");
+ info.setRegion("test-Region");
+ info.setAvailableZone("test-zone");
+ myself.setDataCenterInfo(info);
+
+ MicroserviceInstance allmatchInstance = new MicroserviceInstance();
+ info = new DataCenterInfo();
+ info.setName("test");
+ info.setRegion("test-Region");
+ info.setAvailableZone("test-zone");
+ List<String> allMatchEndpoint = new ArrayList<>();
+ allMatchEndpoint.add("rest://localhost:7090");
+ allmatchInstance.setEndpoints(allMatchEndpoint);
+ allmatchInstance.setDataCenterInfo(info);
+ allmatchInstance.setInstanceId("allmatchInstance");
+ allmatchInstance.setStatus(MicroserviceInstanceStatus.TESTING);
+
+ MicroserviceInstance regionMatchInstance = new MicroserviceInstance();
+ info = new DataCenterInfo();
+ info.setName("test");
+ info.setRegion("test-Region");
+ info.setAvailableZone("test-zone2");
+ List<String> regionMatchEndpoint = new ArrayList<>();
+ regionMatchEndpoint.add("rest://localhost:7091");
+ regionMatchInstance.setEndpoints(regionMatchEndpoint);
+ regionMatchInstance.setDataCenterInfo(info);
+ regionMatchInstance.setInstanceId("regionMatchInstance");
+
+ MicroserviceInstance noneMatchInstance = new MicroserviceInstance();
+ info = new DataCenterInfo();
+ info.setName("test");
+ info.setRegion("test-Region2");
+ info.setAvailableZone("test-zone2");
+ List<String> noMatchEndpoint = new ArrayList<>();
+ noMatchEndpoint.add("rest://localhost:7092");
+ noneMatchInstance.setEndpoints(noMatchEndpoint);
+ noneMatchInstance.setDataCenterInfo(info);
+ noneMatchInstance.setInstanceId("noneMatchInstance");
+
+ Map<String, MicroserviceInstance> data = new HashMap<>();
+ DiscoveryTreeNode parent = new DiscoveryTreeNode().name("parent").data(data);
+ CseContext.getInstance().setTransportManager(transportManager);
+
+ RegistryUtils.setServiceRegistry(serviceRegistry);
+
+ when(serviceRegistry.getMicroserviceInstance()).thenReturn(myself);
+ when(serviceRegistry.getInstanceCacheManager()).thenReturn(instanceCacheManager);
+ when(instanceCacheManager.getOrCreateVersionedCache("testApp", "testMicroserviceName", "0.0.0+"))
+ .thenReturn(parent);
+ when(transportManager.findTransport("rest")).thenReturn(transport);
+
+ LoadbalanceHandler handler = null;
+ LoadBalancer loadBalancer = null;
+ ServiceCombServer server = null;
+
+ DiscoveryTree discoveryTree = new DiscoveryTree();
+ discoveryTree.addFilter(new IsolationDiscoveryFilter());
+ discoveryTree.addFilter(new ZoneAwareDiscoveryFilter());
+ discoveryTree.addFilter(new ServerDiscoveryFilter());
+ discoveryTree.sort();
+ handler = new LoadbalanceHandler(discoveryTree);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server, null);
+
+ data.put("noneMatchInstance", noneMatchInstance);
+ parent.cacheVersion(1);
+ handler = new LoadbalanceHandler();
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7092");
+ handler.handle(invocation, (response) -> {
+ Assert.assertEquals(response.getResult(), "OK");
+ });
+
+ data.put("regionMatchInstance", regionMatchInstance);
+ parent.cacheVersion(parent.cacheVersion() + 1);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7091");
+ handler.handle(invocation, (response) -> {
+ Assert.assertEquals(response.getResult(), "OK");
+ });
+
+ data.put("allmatchInstance", allmatchInstance);
+ parent.cacheVersion(parent.cacheVersion() + 1);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7090");
+
+ ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server);
+ ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server);
+ ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server);
+ ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server);
+ ServiceCombLoadBalancerStats.INSTANCE.markFailure(server);
+
+ //if errorThresholdPercentage is 0,that means errorThresholdPercentage is not active.
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.errorThresholdPercentage", "0");
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7090");
+
+ //if errorThresholdPercentage greater than 0, it will activate.
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.errorThresholdPercentage", "20");
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.minIsolationTime", "10");
+ ServiceCombServer server2 = server;
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7091");
+ ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server2);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7091");
+ TimeUnit.MILLISECONDS.sleep(20);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7090");
+ ServiceCombLoadBalancerStats.INSTANCE.markFailure(server2);
+ ServiceCombLoadBalancerStats.INSTANCE.markFailure(server2);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(), "rest://localhost:7091");
+ handler.handle(invocation, (response) -> {
+ Assert.assertEquals(response.getResult(), "OK");
+ });
+ }
+
@Test
public void testConfigEndpoint() {
ReferenceConfig referenceConfig = Mockito.mock(ReferenceConfig.class);
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/cache/InstanceCache.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/cache/InstanceCache.java
index 20583e833..98bd147b9 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/cache/InstanceCache.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/cache/InstanceCache.java
@@ -100,7 +100,7 @@ public boolean cacheChanged(InstanceCache newCache) {
protected Map<String, List<CacheEndpoint>> createTransportMap() {
Map<String, List<CacheEndpoint>> transportMap = new HashMap<>();
for (MicroserviceInstance instance : instanceMap.values()) {
- // 过滤到不可用实例
+ // This is only used for service center, not change it now
if (instance.getStatus() != MicroserviceInstanceStatus.UP) {
continue;
}
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 e85c51311..bf7269ff6 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
@@ -28,7 +28,6 @@
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;
@@ -198,7 +197,6 @@ private void setInstances(List<MicroserviceInstance> pulledInstances, String rev
List<MicroserviceInstance> inUseInstances) {
List<MicroserviceInstance> upInstances = pulledInstances
.stream()
- .filter(instance -> MicroserviceInstanceStatus.UP.equals(instance.getStatus()))
.collect(Collectors.toList());
if (upInstances.isEmpty() && inUseInstances != null && ServiceRegistryConfig.INSTANCE
.isEmptyInstanceProtectionEnabled()) {
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractDiscoveryFilter.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractDiscoveryFilter.java
index f5cb20203..21dcaa888 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractDiscoveryFilter.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractDiscoveryFilter.java
@@ -17,7 +17,14 @@
package org.apache.servicecomb.serviceregistry.discovery;
+import java.util.HashMap;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
public abstract class AbstractDiscoveryFilter implements DiscoveryFilter {
+ private static final Logger LOGGER = LoggerFactory.getLogger(AbstractDiscoveryFilter.class);
+
@Override
public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode parent) {
if (!parent.childrenInited()) {
@@ -30,7 +37,12 @@ public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode p
}
String childName = findChildName(context, parent);
- return parent.child(childName);
+ DiscoveryTreeNode node = parent.child(childName);
+ if (node == null) {
+ LOGGER.warn("discovery filter {} return null.", this.getClass().getName());
+ return new DiscoveryTreeNode().subName(parent, "empty").data(new HashMap<>());
+ }
+ return node;
}
protected abstract void init(DiscoveryContext context, DiscoveryTreeNode parent);
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/InstanceStatusDiscoveryFilter.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/InstanceStatusDiscoveryFilter.java
new file mode 100644
index 000000000..630a42541
--- /dev/null
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/InstanceStatusDiscoveryFilter.java
@@ -0,0 +1,74 @@
+/*
+ * 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.discovery;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.netflix.config.DynamicPropertyFactory;
+
+public class InstanceStatusDiscoveryFilter extends AbstractDiscoveryFilter {
+ private static final Logger LOGGER = LoggerFactory.getLogger(InstanceStatusDiscoveryFilter.class);
+
+ private static final String UP_INSTANCES = "upInstances";
+
+ @Override
+ public int getOrder() {
+ // OperationInstancesDiscoveryFilter should execute first
+ return -20000;
+ }
+
+ @Override
+ public boolean enabled() {
+ return DynamicPropertyFactory.getInstance()
+ .getBooleanProperty("servicecomb.loadbalance.filter.status.enabled", true).get();
+ }
+
+ @Override
+ public boolean isGroupingFilter() {
+ return true;
+ }
+
+ @Override
+ protected String findChildName(DiscoveryContext context, DiscoveryTreeNode parent) {
+ return UP_INSTANCES;
+ }
+
+ @Override
+ public void init(DiscoveryContext context, DiscoveryTreeNode parent) {
+ Map<String, MicroserviceInstance> instances = parent.data();
+ Map<String, MicroserviceInstance> filteredServers = new HashMap<>();
+ for (String key : instances.keySet()) {
+ MicroserviceInstance instance = instances.get(key);
+ if (MicroserviceInstanceStatus.UP == instance.getStatus()) {
+ filteredServers.put(key, instance);
+ }
+ }
+
+ if (filteredServers.isEmpty()) {
+ return;
+ }
+ DiscoveryTreeNode child = new DiscoveryTreeNode().subName(parent, UP_INSTANCES).data(filteredServers);
+ parent.child(UP_INSTANCES, child);
+ }
+}
diff --git a/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter b/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter
new file mode 100644
index 000000000..3e2418c7e
--- /dev/null
+++ b/service-registry/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter
@@ -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.discovery.InstanceStatusDiscoveryFilter
\ No newline at end of file
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 1382bf9e2..c265634bd 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
@@ -30,7 +30,6 @@
import org.apache.servicecomb.serviceregistry.api.MicroserviceKey;
import org.apache.servicecomb.serviceregistry.api.registry.Microservice;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
-import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus;
import org.apache.servicecomb.serviceregistry.api.response.FindInstancesResponse;
import org.apache.servicecomb.serviceregistry.api.response.MicroserviceInstanceChangedEvent;
import org.apache.servicecomb.serviceregistry.client.http.MicroserviceInstances;
@@ -254,21 +253,6 @@ public void setInstancesMatch() {
Assert.assertSame(instances.get(0), microserviceVersionRule.getInstances().get("i1"));
}
- @Test
- public void setInstances_selectUp() {
- String microserviceId = "1";
-
- createMicroservice(microserviceId);
- createInstance(microserviceId);
- createMicroserviceInstances();
-
- instances.get(0).setStatus(MicroserviceInstanceStatus.DOWN);
- Deencapsulation.invoke(microserviceVersions, "setInstances", instances, "0");
-
- List<?> resultInstances = Deencapsulation.getField(microserviceVersions, "instances");
- Assert.assertTrue(resultInstances.isEmpty());
- }
-
@Test
public void getOrCreateMicroserviceVersionRule() {
MicroserviceVersionRule microserviceVersionRule = microserviceVersions.getOrCreateMicroserviceVersionRule("1.0.0");
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services