You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2022/07/27 06:54:34 UTC
[servicecomb-java-chassis] branch master updated: [SCB-2643]fix instance isolation filter not work problem (#3223)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new 921312cfd [SCB-2643]fix instance isolation filter not work problem (#3223)
921312cfd is described below
commit 921312cfd066a287dee10ffbee4241acfdb5a025
Author: zhaozw <zh...@163.com>
AuthorDate: Wed Jul 27 14:54:29 2022 +0800
[SCB-2643]fix instance isolation filter not work problem (#3223)
---
.../loadbalance/ServerListFilterExt.java | 14 ++-
.../loadbalance/event/IsolationServerEvent.java | 2 +-
.../filter/ZoneAwareDiscoveryFilter.java | 132 -------------------
.../IsolationDiscoveryFilter.java | 83 +++++-------
.../filterext/ZoneAwareDiscoveryFilter.java | 88 +++++++++++++
...he.servicecomb.loadbalance.ServerListFilterExt} | 6 +-
....servicecomb.registry.discovery.DiscoveryFilter | 2 -
.../loadbalance/TestLoadBalanceHandler2.java | 13 +-
.../filter/IsolationDiscoveryFilterTest.java | 139 +++++++++------------
9 files changed, 196 insertions(+), 283 deletions(-)
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 9dba0046d..002bb3a92 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 @@ import org.apache.servicecomb.core.Invocation;
* 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/event/IsolationServerEvent.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/event/IsolationServerEvent.java
index d4f7c6d72..50a51d692 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,7 +20,7 @@ import org.apache.servicecomb.core.Endpoint;
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.registry.api.registry.MicroserviceInstance;
public class IsolationServerEvent extends AlarmEvent {
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 2667c88f8..000000000
--- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/filter/ZoneAwareDiscoveryFilter.java
+++ /dev/null
@@ -1,132 +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.registry.RegistrationManager;
-import org.apache.servicecomb.registry.api.registry.MicroserviceInstance;
-import org.apache.servicecomb.registry.discovery.AbstractDiscoveryFilter;
-import org.apache.servicecomb.registry.discovery.DiscoveryContext;
-import org.apache.servicecomb.registry.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 = RegistrationManager.INSTANCE.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.forEach((id, target) -> {
- 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 68%
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 33650fdcf..8a3f15215 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.registry.api.registry.MicroserviceInstance;
-import org.apache.servicecomb.registry.discovery.DiscoveryContext;
-import org.apache.servicecomb.registry.discovery.DiscoveryFilter;
-import org.apache.servicecomb.registry.discovery.DiscoveryTreeNode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -42,17 +39,15 @@ import com.netflix.config.DynamicPropertyFactory;
/**
* 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);
+ private final EventBus eventBus = EventManager.getEventBus();
+
public static class Settings {
public int errorThresholdPercentage;
@@ -65,11 +60,9 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter {
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,39 +75,25 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter {
@Override
public boolean enabled() {
return DynamicPropertyFactory.getInstance()
- .getBooleanProperty("servicecomb.loadbalance.filter.isolation.enabled", true).get();
- }
-
- @Override
- public boolean isGroupingFilter() {
- return false;
+ .getBooleanProperty(ISOLATION_FILTER_ENABLED, true)
+ .get();
}
@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.forEach((key, instance) -> {
- if (allowVisit(invocation, instance)) {
- filteredServers.put(key, 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) {
@@ -131,14 +110,8 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter {
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) {
@@ -148,10 +121,11 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter {
// 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;
}
@@ -162,10 +136,11 @@ public class IsolationDiscoveryFilter implements DiscoveryFilter {
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 000000000..d31597b5e
--- /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.registry.RegistrationManager;
+import org.apache.servicecomb.registry.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 = RegistrationManager.INSTANCE.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.registry.discovery.DiscoveryFilter b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt
similarity index 73%
copy from handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.registry.discovery.DiscoveryFilter
copy to handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt
index 3a225619b..9e8a08479 100644
--- a/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.registry.discovery.DiscoveryFilter
+++ b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.loadbalance.ServerListFilterExt
@@ -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
+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.registry.discovery.DiscoveryFilter b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.registry.discovery.DiscoveryFilter
index 3a225619b..445ab0efd 100644
--- a/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.registry.discovery.DiscoveryFilter
+++ b/handlers/handler-loadbalance/src/main/resources/META-INF/services/org.apache.servicecomb.registry.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 f4425111d..56cd93f70 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
@@ -17,8 +17,6 @@
package org.apache.servicecomb.loadbalance;
-import static org.mockito.Mockito.when;
-
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@@ -41,9 +39,7 @@ import org.apache.servicecomb.foundation.common.Holder;
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.filter.ServerDiscoveryFilter;
-import org.apache.servicecomb.loadbalance.filter.ZoneAwareDiscoveryFilter;
import org.apache.servicecomb.localregistry.LocalRegistryStore;
import org.apache.servicecomb.registry.DiscoveryManager;
import org.apache.servicecomb.registry.api.registry.DataCenterInfo;
@@ -66,6 +62,8 @@ import com.google.common.eventbus.Subscribe;
import mockit.Mock;
import mockit.MockUp;
+import static org.mockito.Mockito.when;
+
public class TestLoadBalanceHandler2 {
private Holder<Long> mockTimeMillis;
@@ -536,8 +534,6 @@ public class TestLoadBalanceHandler2 {
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);
@@ -669,8 +665,6 @@ public class TestLoadBalanceHandler2 {
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);
@@ -846,12 +840,13 @@ public class TestLoadBalanceHandler2 {
});
Assertions.assertTrue(ServiceCombServerStats.applyForTryingChance(invocation));
- invocation.addLocalContext(IsolationDiscoveryFilter.TRYING_INSTANCES_EXISTING, true);
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "false");
try {
handler.handle(invocation, (response) -> Assertions.assertEquals("OK", response.getResult()));
} catch (Exception e) {
Assertions.fail("unexpected exception " + e.getMessage());
}
+ ArchaiusUtils.setProperty("servicecomb.loadbalance.filter.isolation.enabled", "true");
Assertions.assertEquals("rest://127.0.0.1:8080", invocation.getEndpoint().getEndpoint());
Assertions.assertTrue(serviceCombServerStats.isIsolated());
Assertions.assertEquals(0, serviceCombServerStats.getContinuousFailureCount());
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 bba4e42e6..78b4d0cfd 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,12 +28,9 @@ import org.apache.servicecomb.loadbalance.ServiceCombLoadBalancerStats;
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.registry.api.registry.MicroserviceInstance;
import org.apache.servicecomb.registry.cache.CacheEndpoint;
-import org.apache.servicecomb.registry.discovery.DiscoveryContext;
-import org.apache.servicecomb.registry.discovery.DiscoveryTreeNode;
-import org.hamcrest.MatcherAssert;
-import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -47,14 +44,10 @@ import mockit.Mocked;
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
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);
@@ -67,22 +60,18 @@ public class IsolationDiscoveryFilterTest {
@BeforeEach
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(invocation.getMicroserviceName(), transport,
new CacheEndpoint(endpoint, instance));
+ servers.add(serviceCombServer);
ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(serviceCombServer);
}
- discoveryTreeNode.data(data);
filter = new IsolationDiscoveryFilter();
TestServiceCombServerStats.releaseTryingChance();
@@ -96,43 +85,40 @@ public class IsolationDiscoveryFilterTest {
@Test
public void discoveryNoInstanceReachErrorThreshold() {
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
}
@Test
public void discoveryIsolateErrorInstance() {
- 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();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
// by default 5 times continuous failure will cause isolation
ServiceCombLoadBalancerStats.INSTANCE.markFailure(server0);
Assertions.assertFalse(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 2);
+ Assertions.assertEquals(servers.get(1), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(1));
Assertions.assertTrue(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
}
@Test
public void discoveryTryIsolatedInstanceAfterSingleTestTime() {
- 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) {
@@ -143,12 +129,11 @@ public class IsolationDiscoveryFilterTest {
Assertions.assertTrue(ServiceCombServerStats.isolatedServerCanTry());
Assertions.assertNull(TestServiceCombServerStats.getTryingIsolatedServerInvocation());
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
Assertions.assertTrue(serviceCombServerStats.isIsolated());
Assertions.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
Assertions.assertSame(invocation, TestServiceCombServerStats.getTryingIsolatedServerInvocation());
@@ -156,7 +141,7 @@ public class IsolationDiscoveryFilterTest {
@Test
public void discoveryNotTryIsolatedInstanceConcurrently() {
- 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) {
@@ -168,31 +153,28 @@ public class IsolationDiscoveryFilterTest {
Assertions.assertTrue(ServiceCombServerStats.isolatedServerCanTry());
// The first invocation can occupy the trying chance
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
Assertions.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
// Other invocation cannot get trying chance concurrently
- childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 2);
+ Assertions.assertEquals(servers.get(1), filteredServers.get(0));
+ Assertions.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();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
Assertions.assertFalse(ServiceCombServerStats.isolatedServerCanTry());
}
@@ -206,31 +188,29 @@ public class IsolationDiscoveryFilterTest {
@Test
public void discoveryKeepMinIsolationTime() {
- 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();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i1", "i2"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 2);
+ Assertions.assertEquals(servers.get(1), filteredServers.get(0));
+ Assertions.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();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
}
@Test
public void discoveryRecoverInstance() {
- ServiceCombServer server0 = ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServer(data.get("i0"));
+ ServiceCombServer server0 = servers.get(0);
ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server0);
ServiceCombServerStats serviceCombServerStats = ServiceCombLoadBalancerStats.INSTANCE
.getServiceCombServerStats(server0);
@@ -239,12 +219,11 @@ public class IsolationDiscoveryFilterTest {
Deencapsulation.setField(serviceCombServerStats, "isolatedTime",
System.currentTimeMillis() - Configuration.INSTANCE.getMinIsolationTime(invocation.getMicroserviceName()) - 1);
- DiscoveryTreeNode childNode = filter.discovery(discoveryContext, discoveryTreeNode);
- Map<String, MicroserviceInstance> childNodeData = childNode.data();
- MatcherAssert.assertThat(childNodeData.keySet(), Matchers.containsInAnyOrder("i0", "i1", "i2"));
- Assertions.assertEquals(data.get("i0"), childNodeData.get("i0"));
- Assertions.assertEquals(data.get("i1"), childNodeData.get("i1"));
- Assertions.assertEquals(data.get("i2"), childNodeData.get("i2"));
+ List<ServiceCombServer> filteredServers = filter.getFilteredListOfServers(servers, invocation);
+ Assertions.assertEquals(filteredServers.size(), 3);
+ Assertions.assertEquals(servers.get(0), filteredServers.get(0));
+ Assertions.assertEquals(servers.get(1), filteredServers.get(1));
+ Assertions.assertEquals(servers.get(2), filteredServers.get(2));
Assertions.assertFalse(ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server0).isIsolated());
}
}