[SCB-2643]fix instance isolation filter not work problem (#3310)
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/BeansHolder.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/BeansHolder.java
index d70bfb2..bb5c4db 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/BeansHolder.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/BeansHolder.java
@@ -20,9 +20,6 @@
import javax.inject.Inject;
-import org.springframework.stereotype.Component;
-
-@Component
public class BeansHolder {
@Inject
private List<ExtensionsFactory> extentionsFactories;
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancer.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancer.java
index aced1f5..4b7dfda 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancer.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancer.java
@@ -34,13 +34,13 @@
public class LoadBalancer {
private static final Logger LOGGER = LoggerFactory.getLogger(LoadBalancer.class);
- private static AtomicInteger id = new AtomicInteger(0);
+ private static final AtomicInteger id = new AtomicInteger(0);
- private RuleExt rule;
+ private final RuleExt rule;
- private LoadBalancerStats lbStats;
+ private final LoadBalancerStats lbStats;
- private String microServiceName;
+ private final String microServiceName;
private List<ServerListFilterExt> filters;
@@ -58,7 +58,7 @@
List<ServiceCombServer> servers = invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST);
int serversCount = servers.size();
for (ServerListFilterExt filterExt : filters) {
- if(!filterExt.enabled()) {
+ if (!filterExt.enabled()) {
continue;
}
servers = filterExt.getFilteredListOfServers(servers, invocation);
@@ -76,6 +76,8 @@
LOGGER.info("The Service {}'s instance {} has been isolated for a while, give a single test opportunity.",
invocation.getMicroserviceName(),
server.getInstance().getInstanceId());
+ LOGGER.info("stats: {}-{}-{}-{}", serverStats.getTotalRequests(), serverStats.getSuccessRequests(),
+ serverStats.getFailedRequests(), serverStats.getContinuousFailureCount());
}
return server;
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RandomRuleExt.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RandomRuleExt.java
index 6671d81..5f8d26c 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RandomRuleExt.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RandomRuleExt.java
@@ -18,7 +18,7 @@
package org.apache.servicecomb.loadbalance;
import java.util.List;
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
import org.apache.servicecomb.core.Invocation;
@@ -26,14 +26,12 @@
* A random rule.
*/
public class RandomRuleExt implements RuleExt {
- private Random random = new Random();
-
@Override
public ServiceCombServer choose(List<ServiceCombServer> servers, Invocation invocation) {
if (servers.isEmpty()) {
return null;
}
- int index = Math.abs(random.nextInt()) % servers.size();
+ int index = Math.abs(ThreadLocalRandom.current().nextInt()) % servers.size();
return servers.get(index);
}
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RoundRobinRuleExt.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RoundRobinRuleExt.java
index ce51039..70d4678 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RoundRobinRuleExt.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/RoundRobinRuleExt.java
@@ -26,7 +26,7 @@
* A round robin rule
*/
public class RoundRobinRuleExt implements RuleExt {
- private AtomicInteger counter = new AtomicInteger(0);
+ private final AtomicInteger counter = new AtomicInteger(0);
@Override
public ServiceCombServer choose(List<ServiceCombServer> servers, Invocation invocation) {
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServerListFilterExt.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServerListFilterExt.java
index 9dba004..002bb3a 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServerListFilterExt.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServerListFilterExt.java
@@ -28,8 +28,20 @@
* support this.
*/
public interface ServerListFilterExt {
+ int ORDER_NORMAL = 0;
+
+ int ORDER_ISOLATION = -100;
+
+ int ORDER_ZONE_AWARE = 200;
+
+ String EMPTY_INSTANCE_PROTECTION = "servicecomb.loadbalance.filter.isolation.emptyInstanceProtectionEnabled";
+
+ String ISOLATION_FILTER_ENABLED = "servicecomb.loadbalance.filter.isolation.enabled";
+
+ String ZONE_AWARE_FILTER_ENABLED = "servicecomb.loadbalance.filter.zoneaware.enabled";
+
default int getOrder() {
- return 0;
+ return ORDER_NORMAL;
}
default boolean enabled() {
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServer.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServer.java
index b31e759..aeb63c8 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServer.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServer.java
@@ -20,6 +20,7 @@
import java.net.URI;
import java.net.URISyntaxException;
+import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.core.Endpoint;
import org.apache.servicecomb.core.Transport;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
@@ -90,24 +91,28 @@
return instance;
}
+ @Override
public String toString() {
return endpoint.getEndpoint();
}
// used in LoadBalancerContext
+ @Override
public String getHost() {
return endpoint.getEndpoint();
}
- // take endpoints that belongs to same instance as same server
+ @Override
public boolean equals(Object o) {
if (o instanceof ServiceCombServer) {
- return this.instance.getInstanceId().equals(((ServiceCombServer) o).instance.getInstanceId());
+ return this.instance.getInstanceId().equals(((ServiceCombServer) o).instance.getInstanceId())
+ && StringUtils.equals(endpoint.getEndpoint(), ((ServiceCombServer) o).getEndpoint().getEndpoint());
} else {
return false;
}
}
+ @Override
public int hashCode() {
return this.instance.getInstanceId().hashCode();
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java
index d867da3..140ad21 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/ServiceCombServerStats.java
@@ -65,7 +65,7 @@
private boolean isolated = false;
- private String microserviceName;
+ private final String microserviceName;
public ServiceCombServerStats(String microserviceName) {
this(microserviceName, TimeUtils.getSystemDefaultZoneClock());
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/SessionStickinessRule.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/SessionStickinessRule.java
index a854a83..7483d9c 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/SessionStickinessRule.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/SessionStickinessRule.java
@@ -19,6 +19,7 @@
import java.util.List;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.servicecomb.core.Invocation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -39,7 +40,7 @@
private LoadBalancer loadBalancer;
// use random rule as the trigger rule, to prevent consumer instance select the same producer instance.
- private RuleExt triggerRule;
+ private final RuleExt triggerRule;
private volatile ServiceCombServer lastServer = null;
@@ -75,7 +76,8 @@
return lastServer;
}
- private ServiceCombServer chooseServerWhenTimeout(List<ServiceCombServer> servers, Invocation invocation) {
+ @VisibleForTesting
+ ServiceCombServer chooseServerWhenTimeout(List<ServiceCombServer> servers, Invocation invocation) {
synchronized (lock) {
if (isTimeOut()) {
chooseNextServer(servers, invocation);
@@ -106,9 +108,9 @@
if (stats != null && stats.getServerStats() != null && stats.getServerStats().size() > 0) {
ServerStats serverStats = stats.getSingleServerStat(lastServer);
- int successiveFaildCount = serverStats.getSuccessiveConnectionFailureCount();
+ int successiveFailedCount = serverStats.getSuccessiveConnectionFailureCount();
if (Configuration.INSTANCE.getSuccessiveFailedTimes(microserviceName) > 0
- && successiveFaildCount >= Configuration.INSTANCE.getSuccessiveFailedTimes(microserviceName)) {
+ && successiveFailedCount >= Configuration.INSTANCE.getSuccessiveFailedTimes(microserviceName)) {
serverStats.clearSuccessiveConnectionFailureCount();
return true;
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/WeightedResponseTimeRuleExt.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/WeightedResponseTimeRuleExt.java
index 529c0a3..7a6fe6c 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/WeightedResponseTimeRuleExt.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/WeightedResponseTimeRuleExt.java
@@ -19,7 +19,7 @@
import java.util.ArrayList;
import java.util.List;
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
import org.apache.servicecomb.core.Invocation;
@@ -34,11 +34,9 @@
private static final int RANDOM_PERCENT = 10;
- private Random random = new Random();
-
private LoadBalancer loadBalancer;
- private double totalWeightsCache = 0d;
+ private double totalWeightsCache = -1d;
@Override
public void setLoadBalancer(LoadBalancer loadBalancer) {
@@ -55,7 +53,7 @@
for (int i = 0; i < stats.size() - 1; i++) {
weights.add(finalTotal - stats.get(i));
}
- double ran = random.nextDouble() * finalTotal * (servers.size() - 1);
+ double ran = ThreadLocalRandom.current().nextDouble() * finalTotal * (servers.size() - 1);
for (int i = 0; i < weights.size(); i++) {
ran -= weights.get(i);
if (ran < 0) {
@@ -68,11 +66,11 @@
}
private List<Double> calculateTotalWeights(List<ServiceCombServer> servers) {
- if (totalWeightsCache > MIN_GAP * servers.size()) {
+ if (Double.compare(totalWeightsCache, 0) < 0 || Double.compare(totalWeightsCache, MIN_GAP * servers.size()) > 0) {
return doCalculateTotalWeights(servers);
}
// 10% possibilities to use weighed response rule when the normal access is very fast.
- if (random.nextInt(RANDOM_PERCENT) == 0) {
+ if (ThreadLocalRandom.current().nextInt(RANDOM_PERCENT) == 0) {
return doCalculateTotalWeights(servers);
} else {
return new ArrayList<>();
@@ -85,7 +83,8 @@
boolean needRandom = false;
for (ServiceCombServer server : servers) {
ServerStats serverStats = loadBalancer.getLoadBalancerStats().getSingleServerStat(server);
- double avgTime = serverStats.getResponseTimeAvg();
+ //getResponseTimeAvgRecent()按照时间窗口统计，时间窗口大小为1分钟；getResponseTimeAvg()一直累积
+ double avgTime = serverStats.getResponseTimeAvgRecent();
if (!needRandom && avgTime > MIN_GAP) {
needRandom = true;
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java
index 45c7ba0..abd7396 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java
@@ -20,35 +20,35 @@
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.foundation.common.event.AlarmEvent;
import org.apache.servicecomb.loadbalance.ServiceCombServerStats;
-import org.apache.servicecomb.loadbalance.filter.IsolationDiscoveryFilter;
+import org.apache.servicecomb.loadbalance.filterext.IsolationDiscoveryFilter;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
public class IsolationServerEvent extends AlarmEvent {
- private String microserviceName;
+ private final String microserviceName;
- private Endpoint endpoint;
+ private final Endpoint endpoint;
- private MicroserviceInstance instance;
+ private final MicroserviceInstance instance;
//当前实例总请求数
- private long currentTotalRequest;
+ private final long currentTotalRequest;
//当前实例连续出错次数
- private long currentCountinuousFailureCount;
+ private final long currentCountinuousFailureCount;
//当前实例出错百分比
- private double currentErrorPercentage;
+ private final double currentErrorPercentage;
- private int minIsolationTime;
+ private final int minIsolationTime;
- private long enableRequestThreshold;
+ private final long enableRequestThreshold;
- private int continuousFailureThreshold;
+ private final int continuousFailureThreshold;
- private int errorThresholdPercentage;
+ private final int errorThresholdPercentage;
- private long singleTestTime;
+ private final long singleTestTime;
public IsolationServerEvent(Invocation invocation, MicroserviceInstance instance,
ServiceCombServerStats serverStats,
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/exception/LoadbalanceExceptionUtils.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/exception/LoadbalanceExceptionUtils.java
index 62c2db4..3a4bde5 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/exception/LoadbalanceExceptionUtils.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/exception/LoadbalanceExceptionUtils.java
@@ -30,7 +30,6 @@
public static CseException createLoadbalanceException(String code, Throwable cause, Object... args) {
String msg = String.format(ERROR_DESC_MGR.ensureFindValue(code), args);
- CseException exception = new CseException(code, msg, cause);
- return exception;
+ return new CseException(code, msg, cause);
}
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/InstancePropertyDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/InstancePropertyDiscoveryFilter.java
index 0c1d27a..e8ce416 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/InstancePropertyDiscoveryFilter.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/InstancePropertyDiscoveryFilter.java
@@ -59,10 +59,9 @@
Map<String, MicroserviceInstance> instances = parent.data();
Map<String, String> filterOptions =
Configuration.INSTANCE.getFlowsplitFilterOptions(invocation.getMicroserviceName());
- instances.entrySet().forEach(stringMicroserviceInstanceEntry -> {
- MicroserviceInstance target = stringMicroserviceInstanceEntry.getValue();
+ instances.forEach((key, target) -> {
if (allowVisit(target, filterOptions)) {
- matchedInstance.put(stringMicroserviceInstanceEntry.getKey(), target);
+ matchedInstance.put(key, target);
}
});
parent.child(MATCHED, new DiscoveryTreeNode()
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/PriorityInstancePropertyDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/PriorityInstancePropertyDiscoveryFilter.java
index d3758cd..2a5d8a9 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/PriorityInstancePropertyDiscoveryFilter.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/PriorityInstancePropertyDiscoveryFilter.java
@@ -93,7 +93,7 @@
currentProperty = currentProperty.child();
}
}
- LOGGER.debug("Discovery instance filter by {}", currentProperty.toString());
+ LOGGER.debug("Discovery instance filter by {}", currentProperty);
context.putContextParameter(propertyKey, currentProperty);
// stop push filter stack if property is empty
@@ -116,7 +116,7 @@
return new InstancePropertyDiscoveryFilter().getOrder() + 1;
}
- class PriorityInstanceProperty {
+ static class PriorityInstanceProperty {
private static final int MAX_LENGTH = 10000;
private static final String SEPARATOR = ".";
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/ZoneAwareDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/ZoneAwareDiscoveryFilter.java
deleted file mode 100644
index c444467..0000000
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/ZoneAwareDiscoveryFilter.java
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
- * 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.loadbalance.filter;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
-import org.apache.servicecomb.serviceregistry.RegistryUtils;
-import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
-import org.apache.servicecomb.serviceregistry.discovery.AbstractDiscoveryFilter;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTreeNode;
-
-import com.netflix.config.DynamicPropertyFactory;
-
-
-public class ZoneAwareDiscoveryFilter extends AbstractDiscoveryFilter {
- public static final String KEY_ZONE_AWARE_STEP = "_KEY_ZONE_AWARE_STEP";
-
- private static final String GROUP_RegionAndAZMatch = "instancesRegionAndAZMatch";
-
- private static final String GROUP_InstancesAZMatch = "instancesAZMatch";
-
- private static final String GROUP_InstancesNoMatch = "instancesNoMatch";
-
- public static final String GROUP_Instances_All = "instancesAll";
-
- @Override
- public int getOrder() {
- return 300;
- }
-
- @Override
- public boolean enabled() {
- return DynamicPropertyFactory.getInstance()
- .getBooleanProperty("servicecomb.loadbalance.filter.zoneaware.enabled", true).get();
- }
-
- @Override
- public boolean isGroupingFilter() {
- return true;
- }
-
- @Override
- protected void init(DiscoveryContext context, DiscoveryTreeNode parent) {
- MicroserviceInstance myself = RegistryUtils.getMicroserviceInstance();
-
- Map<String, MicroserviceInstance> instancesRegionAndAZMatch = new HashMap<>();
- Map<String, MicroserviceInstance> instancesAZMatch = new HashMap<>();
- Map<String, MicroserviceInstance> instancesNoMatch = new HashMap<>();
- Map<String, MicroserviceInstance> instances = parent.data();
- instances.entrySet().forEach(stringMicroserviceInstanceEntry -> {
- String id = stringMicroserviceInstanceEntry.getKey();
- MicroserviceInstance target = stringMicroserviceInstanceEntry.getValue();
- if (regionAndAZMatch(myself, target)) {
- instancesRegionAndAZMatch.put(id, target);
- } else if (regionMatch(myself, target)) {
- instancesAZMatch.put(id, target);
- } else {
- instancesNoMatch.put(id, target);
- }
- });
- Map<String, DiscoveryTreeNode> children = new HashMap<>();
- children.put(GROUP_RegionAndAZMatch, new DiscoveryTreeNode()
- .subName(parent, GROUP_RegionAndAZMatch)
- .data(instancesRegionAndAZMatch));
- children.put(GROUP_InstancesAZMatch, new DiscoveryTreeNode()
- .subName(parent, GROUP_InstancesAZMatch)
- .data(instancesAZMatch));
- children.put(GROUP_InstancesNoMatch, new DiscoveryTreeNode()
- .subName(parent, GROUP_InstancesNoMatch)
- .data(instancesNoMatch));
- children.put(GROUP_Instances_All, new DiscoveryTreeNode()
- .subName(parent, GROUP_Instances_All)
- .data(instances));
- parent.children(children);
- }
-
- @Override
- protected String findChildName(DiscoveryContext context, DiscoveryTreeNode parent) {
- String key = context.getContextParameter(KEY_ZONE_AWARE_STEP);
- if (key == null) {
- key = GROUP_RegionAndAZMatch;
- context.pushRerunFilter();
- } else if (GROUP_RegionAndAZMatch.equals(key)) {
- key = GROUP_InstancesAZMatch;
- context.pushRerunFilter();
- } else if (GROUP_InstancesAZMatch.equals(key)) {
- key = GROUP_InstancesNoMatch;
- context.pushRerunFilter();
- } else if (GROUP_InstancesNoMatch.equals(key)) {
- key = GROUP_Instances_All;
- } else {
- throw new ServiceCombException("not possible happen, maybe a bug.");
- }
- context.putContextParameter(KEY_ZONE_AWARE_STEP, key);
- return key;
- }
-
- private boolean regionAndAZMatch(MicroserviceInstance myself, MicroserviceInstance target) {
- if (myself.getDataCenterInfo() == null) {
- // when instance have no datacenter info, it will match all other datacenters
- return true;
- }
- if (target.getDataCenterInfo() != null) {
- return myself.getDataCenterInfo().getRegion().equals(target.getDataCenterInfo().getRegion()) &&
- myself.getDataCenterInfo().getAvailableZone().equals(target.getDataCenterInfo().getAvailableZone());
- }
- return false;
- }
-
- private boolean regionMatch(MicroserviceInstance myself, MicroserviceInstance target) {
- if (target.getDataCenterInfo() != null) {
- return myself.getDataCenterInfo().getRegion().equals(target.getDataCenterInfo().getRegion());
- }
- return false;
- }
-}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/IsolationDiscoveryFilter.java
similarity index 66%
rename from handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java
rename to handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/IsolationDiscoveryFilter.java
index 7919c1d..8a3f152 100644
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilter.java
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/IsolationDiscoveryFilter.java
@@ -15,23 +15,20 @@
* limitations under the License.
*/
-package org.apache.servicecomb.loadbalance.filter;
+package org.apache.servicecomb.loadbalance.filterext;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.foundation.common.event.AlarmEvent.Type;
import org.apache.servicecomb.foundation.common.event.EventManager;
import org.apache.servicecomb.loadbalance.Configuration;
+import org.apache.servicecomb.loadbalance.ServerListFilterExt;
import org.apache.servicecomb.loadbalance.ServiceCombLoadBalancerStats;
import org.apache.servicecomb.loadbalance.ServiceCombServer;
import org.apache.servicecomb.loadbalance.ServiceCombServerStats;
import org.apache.servicecomb.loadbalance.event.IsolationServerEvent;
-import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTreeNode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -42,18 +39,16 @@
/**
* Isolate instances by error metrics
*/
-public class IsolationDiscoveryFilter implements DiscoveryFilter {
-
- public static final String TRYING_INSTANCES_EXISTING = "scb-hasTryingInstances";
+public class IsolationDiscoveryFilter implements ServerListFilterExt {
private static final Logger LOGGER = LoggerFactory.getLogger(IsolationDiscoveryFilter.class);
- private static final String EMPTY_INSTANCE_PROTECTION = "servicecomb.loadbalance.filter.isolation.emptyInstanceProtectionEnabled";
-
private final DynamicBooleanProperty emptyProtection = DynamicPropertyFactory.getInstance()
.getBooleanProperty(EMPTY_INSTANCE_PROTECTION, false);
- public class Settings {
+ private final EventBus eventBus = EventManager.getEventBus();
+
+ public static class Settings {
public int errorThresholdPercentage;
public long singleTestTime;
@@ -65,11 +60,9 @@
public int minIsolationTime; // to avoid isolation recover too fast due to no concurrent control in concurrent scenario
}
- public EventBus eventBus = EventManager.getEventBus();
-
@Override
public int getOrder() {
- return 500;
+ return ORDER_ISOLATION;
}
public IsolationDiscoveryFilter() {
@@ -82,40 +75,25 @@
@Override
public boolean enabled() {
return DynamicPropertyFactory.getInstance()
- .getBooleanProperty("servicecomb.loadbalance.filter.isolation.enabled", true).get();
+ .getBooleanProperty(ISOLATION_FILTER_ENABLED, true)
+ .get();
}
@Override
- public boolean isGroupingFilter() {
- return false;
- }
-
- @Override
- public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode parent) {
- Map<String, MicroserviceInstance> instances = parent.data();
- Invocation invocation = context.getInputParameters();
- if (!Configuration.INSTANCE.isIsolationFilterOpen(invocation.getMicroserviceName())) {
- return parent;
- }
-
- Map<String, MicroserviceInstance> filteredServers = new HashMap<>();
- instances.entrySet().forEach(stringMicroserviceInstanceEntry -> {
- MicroserviceInstance instance = stringMicroserviceInstanceEntry.getValue();
- if (allowVisit(invocation, instance)) {
- filteredServers.put(stringMicroserviceInstanceEntry.getKey(), instance);
+ public List<ServiceCombServer> getFilteredListOfServers(List<ServiceCombServer> servers,
+ Invocation invocation) {
+ List<ServiceCombServer> filteredServers = new ArrayList<>();
+ Settings settings = createSettings(invocation);
+ servers.forEach((server) -> {
+ if (allowVisit(invocation, server, settings)) {
+ filteredServers.add(server);
}
});
-
- DiscoveryTreeNode child = parent.children().computeIfAbsent("filterred", etn -> new DiscoveryTreeNode());
- if (ZoneAwareDiscoveryFilter.GROUP_Instances_All
- .equals(context.getContextParameter(ZoneAwareDiscoveryFilter.KEY_ZONE_AWARE_STEP)) && filteredServers.isEmpty()
- && emptyProtection.get()) {
+ if (filteredServers.isEmpty() && emptyProtection.get()) {
LOGGER.warn("All servers have been isolated, allow one of them based on load balance rule.");
- child.data(instances);
- } else {
- child.data(filteredServers);
+ return servers;
}
- return child;
+ return filteredServers;
}
private Settings createSettings(Invocation invocation) {
@@ -132,14 +110,8 @@
return settings;
}
- private boolean allowVisit(Invocation invocation, MicroserviceInstance instance) {
- ServiceCombServer server = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(instance);
- if (server == null) {
- // first time accessed.
- return true;
- }
+ private boolean allowVisit(Invocation invocation, ServiceCombServer server, Settings settings) {
ServiceCombServerStats serverStats = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server);
- Settings settings = createSettings(invocation);
if (!checkThresholdAllowed(settings, serverStats)) {
if (serverStats.isIsolated()
&& (System.currentTimeMillis() - serverStats.getLastVisitTime()) > settings.singleTestTime) {
@@ -149,10 +121,11 @@
// checkThresholdAllowed is not concurrent control, may print several logs/events in current access.
serverStats.markIsolated(true);
eventBus.post(
- new IsolationServerEvent(invocation, instance, serverStats,
+ new IsolationServerEvent(invocation, server.getInstance(), serverStats,
settings, Type.OPEN, server.getEndpoint()));
- LOGGER.warn("Isolate service {}'s instance {}.", invocation.getMicroserviceName(),
- instance.getInstanceId());
+ LOGGER.warn("Isolate service {}'s instance {}.",
+ invocation.getMicroserviceName(),
+ server.getInstance().getInstanceId());
}
return false;
}
@@ -163,10 +136,11 @@
return false;
}
serverStats.markIsolated(false);
- eventBus.post(new IsolationServerEvent(invocation, instance, serverStats,
+ eventBus.post(new IsolationServerEvent(invocation, server.getInstance(), serverStats,
settings, Type.CLOSE, server.getEndpoint()));
- LOGGER.warn("Recover service {}'s instance {} from isolation.", invocation.getMicroserviceName(),
- instance.getInstanceId());
+ LOGGER.warn("Recover service {}'s instance {} from isolation.",
+ invocation.getMicroserviceName(),
+ server.getInstance().getInstanceId());
}
return true;
}
diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/ZoneAwareDiscoveryFilter.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/ZoneAwareDiscoveryFilter.java
new file mode 100644
index 0000000..08fa092
--- /dev/null
+++ b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filterext/ZoneAwareDiscoveryFilter.java
@@ -0,0 +1,88 @@
+/*
+ * 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.loadbalance.filterext;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.servicecomb.core.Invocation;
+import org.apache.servicecomb.loadbalance.ServerListFilterExt;
+import org.apache.servicecomb.loadbalance.ServiceCombServer;
+import org.apache.servicecomb.serviceregistry.RegistryUtils;
+import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
+
+import com.netflix.config.DynamicPropertyFactory;
+
+public class ZoneAwareDiscoveryFilter implements ServerListFilterExt {
+
+ @Override
+ public int getOrder() {
+ return ORDER_ZONE_AWARE;
+ }
+
+ @Override
+ public boolean enabled() {
+ return DynamicPropertyFactory.getInstance()
+ .getBooleanProperty(ZONE_AWARE_FILTER_ENABLED, true)
+ .get();
+ }
+
+ @Override
+ public List<ServiceCombServer> getFilteredListOfServers(List<ServiceCombServer> servers,
+ Invocation invocation) {
+ MicroserviceInstance myself = RegistryUtils.getMicroserviceInstance();
+
+ List<ServiceCombServer> instancesRegionAndAZMatch = new ArrayList<>();
+ List<ServiceCombServer> instancesAZMatch = new ArrayList<>();
+ List<ServiceCombServer> instancesNoMatch = new ArrayList<>();
+ servers.forEach((server) -> {
+ if (regionAndAZMatch(myself, server.getInstance())) {
+ instancesRegionAndAZMatch.add(server);
+ } else if (regionMatch(myself, server.getInstance())) {
+ instancesAZMatch.add(server);
+ } else {
+ instancesNoMatch.add(server);
+ }
+ });
+ if (!instancesRegionAndAZMatch.isEmpty()) {
+ return instancesRegionAndAZMatch;
+ } else if (!instancesAZMatch.isEmpty()) {
+ return instancesAZMatch;
+ }
+ return instancesNoMatch;
+ }
+
+ private boolean regionAndAZMatch(MicroserviceInstance myself, MicroserviceInstance target) {
+ if (myself == null || myself.getDataCenterInfo() == null) {
+ // when instance have no datacenter info, it will match all other datacenters
+ return true;
+ }
+ if (target.getDataCenterInfo() != null) {
+ return myself.getDataCenterInfo().getRegion().equals(target.getDataCenterInfo().getRegion()) &&
+ myself.getDataCenterInfo().getAvailableZone().equals(target.getDataCenterInfo().getAvailableZone());
+ }
+ return false;
+ }
+
+ private boolean regionMatch(MicroserviceInstance myself, MicroserviceInstance target) {
+ if (target.getDataCenterInfo() != null) {
+ return myself.getDataCenterInfo().getRegion().equals(target.getDataCenterInfo().getRegion());
+ }
+ return false;
+ }
+}
diff --git a/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt
new file mode 100644
index 0000000..9e8a084
--- /dev/null
+++ b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt
@@ -0,0 +1,19 @@
+#
+# 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.loadbalance.filterext.IsolationDiscoveryFilter
+org.apache.servicecomb.loadbalance.filterext.ZoneAwareDiscoveryFilter
\ No newline at end of file
diff --git a/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter
index 3a22561..445ab0e 100644
--- a/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter
+++ b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.serviceregistry.discovery.DiscoveryFilter
@@ -15,7 +15,5 @@
# limitations under the License.
#
-org.apache.servicecomb.loadbalance.filter.ZoneAwareDiscoveryFilter
-org.apache.servicecomb.loadbalance.filter.IsolationDiscoveryFilter
org.apache.servicecomb.loadbalance.filter.InstancePropertyDiscoveryFilter
org.apache.servicecomb.loadbalance.filter.PriorityInstancePropertyDiscoveryFilter
\ No newline at end of file
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 fd3effe..bf8a5df 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
@@ -41,9 +41,9 @@
import org.apache.servicecomb.foundation.common.event.EventManager;
import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
import org.apache.servicecomb.loadbalance.event.IsolationServerEvent;
-import org.apache.servicecomb.loadbalance.filter.IsolationDiscoveryFilter;
+import org.apache.servicecomb.loadbalance.filterext.IsolationDiscoveryFilter;
import org.apache.servicecomb.loadbalance.filter.ServerDiscoveryFilter;
-import org.apache.servicecomb.loadbalance.filter.ZoneAwareDiscoveryFilter;
+import org.apache.servicecomb.loadbalance.filterext.ZoneAwareDiscoveryFilter;
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import org.apache.servicecomb.serviceregistry.ServiceRegistry;
import org.apache.servicecomb.serviceregistry.api.registry.DataCenterInfo;
@@ -532,8 +532,6 @@
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);
@@ -676,8 +674,6 @@
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);
@@ -855,12 +851,13 @@
});
Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation));
- invocation.addLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING, true);
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "false");
try {
handler.handle(invocation, (response) -> Assert.assertEquals("OK", response.getResult()));
} catch (Exception e) {
Assert.fail("unexpected exception " + e.getMessage());
}
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "true");
Assert.assertEquals("rest://127.0.0.1:8080", invocation.getEndpoint().getEndpoint());
Assert.assertTrue(serviceCombServerStats.isIsolated());
Assert.assertEquals(0, serviceCombServerStats.getContinuousFailureCount());
@@ -912,12 +909,13 @@
});
Assert.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation));
- invocation.addLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING, true);
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "false");
try {
handler.handle(invocation, (response) -> Assert.assertEquals("OK", response.getResult()));
} catch (Exception e) {
Assert.fail("unexpected exception " + e.getMessage());
}
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "true");
Assert.assertEquals("rest://127.0.0.1:8081", invocation.getEndpoint().getEndpoint());
Assert.assertFalse(stats0.isIsolated());
Assert.assertEquals(1, stats0.getContinuousFailureCount());
diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestWeightedResponseTimeRuleExt.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestWeightedResponseTimeRuleExt.java
index 10dfed3..065f7ba 100644
--- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestWeightedResponseTimeRuleExt.java
+++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestWeightedResponseTimeRuleExt.java
@@ -53,30 +53,52 @@
}
@Test
- public void testWeighed() {
+ public void testWeighed() throws InterruptedException {
WeightedResponseTimeRuleExt rule = new WeightedResponseTimeRuleExt();
LoadBalancer loadBalancer = new LoadBalancer(rule, "testService");
List<ServiceCombServer> servers = new ArrayList<>();
Invocation invocation = Mockito.mock(Invocation.class);
- for (int i = 0; i < 2; i++) {
- ServiceCombServer server = Mockito.mock(ServiceCombServer.class);
- Mockito.when(server.toString()).thenReturn("server " + i);
- servers.add(server);
- loadBalancer.getLoadBalancerStats().noteResponseTime(server, 20 * Math.pow(4, i + 1));
- }
- AtomicInteger server1 = new AtomicInteger(0);
- AtomicInteger server2 = new AtomicInteger(0);
+ ServiceCombServer server1 = Mockito.mock(ServiceCombServer.class);
+ Mockito.when(server1.toString()).thenReturn("server " + 0);
+ servers.add(server1);
+ ServiceCombServer server2 = Mockito.mock(ServiceCombServer.class);
+ Mockito.when(server2.toString()).thenReturn("server " + 1);
+ servers.add(server2);
+
+ AtomicInteger serverCounter1 = new AtomicInteger(0);
+ AtomicInteger serverCounter2 = new AtomicInteger(0);
for (int i = 0; i < 2000; i++) {
+ loadBalancer.getLoadBalancerStats().noteResponseTime(server1, 20);
+ loadBalancer.getLoadBalancerStats().noteResponseTime(server2, 400);
+ Thread.sleep(1);
if (rule.choose(servers, invocation).toString().equals("server 0")) {
- server1.incrementAndGet();
+ serverCounter1.incrementAndGet();
} else {
- server2.incrementAndGet();
+ serverCounter2.incrementAndGet();
}
}
- double percent = (double) server1.get() / (server2.get() + server1.get());
+ double percent = (double) serverCounter1.get() / (serverCounter2.get() + serverCounter1.get());
System.out.println("percent" + percent);
- Assert.assertEquals("actually percent: " + percent, 0.70d < percent, percent < 0.90d);
+ Assert.assertTrue(percent > 0.60d);
+ Assert.assertTrue(percent < 0.90d);
+ serverCounter1.set(0);
+ serverCounter2.set(0);
+
+ Thread.sleep(1000);
+ for (int i = 0; i < 2000; i++) {
+ loadBalancer.getLoadBalancerStats().noteResponseTime(server1, 20);
+ loadBalancer.getLoadBalancerStats().noteResponseTime(server2, 20);
+ Thread.sleep(1);
+ if (rule.choose(servers, invocation).toString().equals("server 0")) {
+ serverCounter1.incrementAndGet();
+ } else {
+ serverCounter2.incrementAndGet();
+ }
+ }
+ percent = (double) serverCounter1.get() / (serverCounter2.get() + serverCounter1.get());
+ System.out.println("percent" + percent);
+ Assert.assertEquals(0.50d, percent, 0.2);
}
@Test
@@ -99,7 +121,7 @@
}
long taken = System.currentTimeMillis() - begin;
System.out.println("taken " + taken);
- Assert.assertEquals("actually taken: " + taken, taken < 1000 * 5, true); // 5 * times make slow machine happy
+ Assert.assertTrue(taken < 1000 * 5); // 5 * times make slow machine happy
}
@Test
@@ -121,6 +143,6 @@
}
long taken = System.currentTimeMillis() - begin;
System.out.println("taken " + taken);
- Assert.assertEquals("actually taken: " + taken, taken < 200 * 5, true); // 5 * times make slow machine happy
+ Assert.assertTrue(taken < 200 * 5); // 5 * times make slow machine happy
}
}
diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java
index 0e16acb..8c8e1d4 100644
--- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java
+++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/filter/IsolationDiscoveryFilterTest.java
@@ -17,9 +17,9 @@
package org.apache.servicecomb.loadbalance.filter;
+import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.List;
import org.apache.servicecomb.core.Invocation;
import org.apache.servicecomb.core.Transport;
@@ -28,11 +28,9 @@
import org.apache.servicecomb.loadbalance.ServiceCombServer;
import org.apache.servicecomb.loadbalance.ServiceCombServerStats;
import org.apache.servicecomb.loadbalance.TestServiceCombServerStats;
+import org.apache.servicecomb.loadbalance.filterext.IsolationDiscoveryFilter;
import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
import org.apache.servicecomb.serviceregistry.cache.CacheEndpoint;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryContext;
-import org.apache.servicecomb.serviceregistry.discovery.DiscoveryTreeNode;
-import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -43,15 +41,10 @@
import mockit.Mocked;
public class IsolationDiscoveryFilterTest {
-
- private DiscoveryContext discoveryContext;
-
- private DiscoveryTreeNode discoveryTreeNode;
-
- private Map<String, MicroserviceInstance> data;
-
private IsolationDiscoveryFilter filter;
+ private List<ServiceCombServer> servers;
+
@Mocked
private Transport transport = Mockito.mock(Transport.class);
@@ -64,22 +57,18 @@
@Before
public void before() {
- discoveryContext = new DiscoveryContext();
- discoveryContext.setInputParameters(invocation);
- discoveryTreeNode = new DiscoveryTreeNode();
Mockito.doAnswer(a -> a.getArguments()[0]).when(transport).parseAddress(Mockito.anyString());
- data = new HashMap<>();
+ servers = new ArrayList<>();
for (int i = 0; i < 3; ++i) {
MicroserviceInstance instance = new MicroserviceInstance();
instance.setInstanceId("i" + i);
String endpoint = "rest://127.0.0.1:" + i;
instance.setEndpoints(Collections.singletonList(endpoint));
- data.put(instance.getInstanceId(), instance);
ServiceCombServer serviceCombServer = new ServiceCombServer(null, transport,
new CacheEndpoint(endpoint, instance));
+ servers.add(serviceCombServer);
ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer);
}
- discoveryTreeNode.data(data);
filter = new IsolationDiscoveryFilter();
TestServiceCombServerStats.releaseTryingChance();
@@ -93,43 +82,40 @@
@Test
public void discovery_no_instance_reach_error_threshold() {
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
}
@Test
public void discovery_isolate_error_instance() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
for (int i = 0; i < 4; ++i) {
ServiceCombLoadBalancerStats.INSTANCE.markFailure(server0);
}
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
// by default 5 times continuous failure will cause isolation
ServiceCombLoadBalancerStats.INSTANCE.markFailure(server0);
Assert.assertFalse(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 2);
+ Assert.assertEquals(servers.get(1), filteredServers.get(0));
+ Assert.assertEquals(servers.get(2), filteredServers.get(1));
Assert.assertTrue(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
}
@Test
public void discovery_try_isolated_instance_after_singleTestTime() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
ServiceCombServerStats serviceCombServerStats = ServiceCombLoadBalancerStats.INSTANCE
.getServiceCombServerStats(server0);
for (int i = 0; i < 5; ++i) {
@@ -140,12 +126,11 @@
Assert.assertTrue(ServiceCombServerStats.isolatedServerCanTry());
Assert.assertNull(TestServiceCombServerStats.getTryingIsolatedServerInvocation());
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
Assert.assertTrue(serviceCombServerStats.isIsolated());
Assert.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
Assert.assertSame(invocation, TestServiceCombServerStats.getTryingIsolatedServerInvocation());
@@ -153,7 +138,7 @@
@Test
public void discovery_not_try_isolated_instance_concurrently() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
ServiceCombServerStats serviceCombServerStats = ServiceCombLoadBalancerStats.INSTANCE
.getServiceCombServerStats(server0);
for (int i = 0; i < 5; ++i) {
@@ -165,32 +150,28 @@
Assert.assertTrue(ServiceCombServerStats.isolatedServerCanTry());
// The first invocation can occupy the trying chance
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
Assert.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
// Other invocation cannot get trying chance concurrently
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 2);
+ Assert.assertEquals(servers.get(1), filteredServers.get(0));
+ Assert.assertEquals(servers.get(2), filteredServers.get(1));
ServiceCombServerStats
.checkAndReleaseTryingChance(invocation); // after the first invocation releases the trying chance
// Other invocation can get the trying chance
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
- Assert.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
}
private ServiceCombServerStats letIsolatedInstancePassSingleTestTime(ServiceCombServerStats serviceCombServerStats) {
@@ -203,31 +184,29 @@
@Test
public void discovery_keep_minIsolationTime() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server0, true);
ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server0);
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 2);
+ Assert.assertEquals(servers.get(1), filteredServers.get(0));
+ Assert.assertEquals(servers.get(2), filteredServers.get(1));
ServiceCombServerStats serviceCombServerStats = ServiceCombLoadBalancerStats.INSTANCE
.getServiceCombServerStats(server0);
Deencapsulation.setField(serviceCombServerStats, "isolatedTime",
System.currentTimeMillis() - Configuration.INSTANCE.getMinIsolationTime(invocation.getMicroserviceName()) - 1);
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
}
@Test
public void discovery_recover_instance() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server0);
ServiceCombServerStats serviceCombServerStats = ServiceCombLoadBalancerStats.INSTANCE
.getServiceCombServerStats(server0);
@@ -236,12 +215,10 @@
Deencapsulation.setField(serviceCombServerStats, "isolatedTime",
System.currentTimeMillis() - Configuration.INSTANCE.getMinIsolationTime(invocation.getMicroserviceName()) - 1);
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- Assert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assert.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assert.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assert.assertEquals(data.get("i2"), childNodeData.get("i2"));
- Assert.assertFalse(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assert.assertEquals(filteredServers.size(), 3);
+ Assert.assertEquals(servers.get(0), filteredServers.get(0));
+ Assert.assertEquals(servers.get(1), filteredServers.get(1));
+ Assert.assertEquals(servers.get(2), filteredServers.get(2));
}
}
\ No newline at end of file
