[
https://issues.apache.org/jira/browse/SCB-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16656152#comment-16656152
]
ASF GitHub Bot commented on SCB-961:
------------------------------------
liubao68 closed pull request #957: [SCB-961]provide a way to isolate for at
least a moment
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/957
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
index e7a88e00d..c6aa4206f 100644
---
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
+++
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/Configuration.java
@@ -66,6 +66,8 @@
public static final String FILTER_SINGLE_TEST = "singleTestTime";
+ public static final String FILTER_MIN_ISOLATION_TIME = "minIsolationTime";
+
public static final String FILTER_CONTINUOUS_FAILURE_THRESHOLD =
"continuousFailureThreshold";
public static final String TRANSACTIONCONTROL_OPTIONS_PREFIX_PATTERN =
@@ -197,7 +199,7 @@ public int getSingleTestTime(String microservice) {
PROP_ROOT + FILTER_ISOLATION + FILTER_SINGLE_TEST);
try {
int result = Integer.parseInt(p);
- if (result > 0) {
+ if (result >= 0) {
return result;
}
return defaultValue;
@@ -207,6 +209,22 @@ public int getSingleTestTime(String microservice) {
}
}
+ public int getMinIsolationTime(String microservice) {
+ final int defaultValue = 3000; // 3 seconds
+ String p = getStringProperty("3000",
+ PROP_ROOT + microservice + "." + FILTER_ISOLATION +
FILTER_MIN_ISOLATION_TIME,
+ PROP_ROOT + FILTER_ISOLATION + FILTER_MIN_ISOLATION_TIME);
+ try {
+ int result = Integer.parseInt(p);
+ if (result >= 0) {
+ return result;
+ }
+ return defaultValue;
+ } catch (NumberFormatException e) {
+ return defaultValue;
+ }
+ }
+
public Map<String, String> getFlowsplitFilterOptions(String microservice) {
String keyPrefix =
String.format(TRANSACTIONCONTROL_OPTIONS_PREFIX_PATTERN, microservice);
return ConfigurePropertyUtils.getPropertiesWithPrefix(keyPrefix);
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/filter/IsolationDiscoveryFilter.java
index 6cfadb79c..f3e5c425a 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/filter/IsolationDiscoveryFilter.java
@@ -53,8 +53,9 @@
long enableRequestThreshold;
int continuousFailureThreshold;
- }
+ int minIsolationTime; // to avoid isolation recover too fast due to no
concurrent control in concurrent scenario
+ }
public EventBus eventBus = EventManager.getEventBus();
@Override
@@ -102,6 +103,8 @@ private Settings createSettings(Invocation invocation) {
.getEnableRequestThreshold(invocation.getMicroserviceName());
settings.continuousFailureThreshold = Configuration.INSTANCE
.getContinuousFailureThreshold(invocation.getMicroserviceName());
+ settings.minIsolationTime = Configuration.INSTANCE
+ .getMinIsolationTime(invocation.getMicroserviceName());
return settings;
}
@@ -114,37 +117,41 @@ private boolean allowVisit(Invocation invocation,
MicroserviceInstance instance)
ServiceCombServerStats serverStats =
ServiceCombLoadBalancerStats.INSTANCE.getServiceCombServerStats(server);
Settings settings = createSettings(invocation);
if (!checkThresholdAllowed(settings, serverStats)) {
- if ((System.currentTimeMillis() - serverStats.getLastVisitTime()) >
settings.singleTestTime) {
- // because we will ping server status, this logic may not be very
likely to happen.
+ if (serverStats.isIsolated()
+ && (System.currentTimeMillis() - serverStats.getLastVisitTime()) >
settings.singleTestTime) {
+ // [1]we can implement better recovery based on several attempts, but
here we do not know if this attempt is success
LOGGER.info("The Service {}'s instance {} has been isolated for a
while, give a single test opportunity.",
invocation.getMicroserviceName(),
instance.getInstanceId());
return true;
}
-
if (!serverStats.isIsolated()) {
- LOGGER.warn("Isolate service {}'s instance {}.",
invocation.getMicroserviceName(),
- instance.getInstanceId());
+ ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server, true);
eventBus.post(
new IsolationServerEvent(invocation.getMicroserviceName(),
serverStats.getTotalRequests(),
serverStats.getCountinuousFailureCount(),
serverStats.getFailedRate(),
settings.continuousFailureThreshold,
settings.errorThresholdPercentage, settings.enableRequestThreshold,
settings.singleTestTime, Type.OPEN));
- ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server, true);
+ LOGGER.warn("Isolate service {}'s instance {}.",
invocation.getMicroserviceName(),
+ instance.getInstanceId());
}
return false;
}
-
if (serverStats.isIsolated()) {
- LOGGER.warn("Recover service {}'s instance {} from isolation.",
invocation.getMicroserviceName(),
- instance.getInstanceId());
+ // [2] so that we add a feature to isolate for at least a minimal time,
and we can avoid
+ // high volume of concurrent requests with a percentage of error(e.g.
50%) scenario with no isolation
+ if ((System.currentTimeMillis() - serverStats.getLastVisitTime()) <=
settings.minIsolationTime) {
+ return false;
+ }
+ ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server, false);
eventBus.post(new IsolationServerEvent(invocation.getMicroserviceName(),
serverStats.getTotalRequests(),
serverStats.getCountinuousFailureCount(),
serverStats.getFailedRate(),
settings.continuousFailureThreshold,
settings.errorThresholdPercentage, settings.enableRequestThreshold,
settings.singleTestTime, Type.CLOSE));
- ServiceCombLoadBalancerStats.INSTANCE.markIsolated(server, false);
+ LOGGER.warn("Recover service {}'s instance {} from isolation.",
invocation.getMicroserviceName(),
+ instance.getInstanceId());
}
return true;
}
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 276e45412..f962c9390 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
@@ -23,6 +23,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.TimeUnit;
import org.apache.servicecomb.core.CseContext;
import org.apache.servicecomb.core.Invocation;
@@ -63,7 +64,7 @@ public void teardown() {
}
@Test
- public void testZoneAwareAndIsolationFilterWorks() {
+ public void testZoneAwareAndIsolationFilterWorks() throws Exception {
ReferenceConfig referenceConfig = Mockito.mock(ReferenceConfig.class);
OperationMeta operationMeta = Mockito.mock(OperationMeta.class);
SchemaMeta schemaMeta = Mockito.mock(SchemaMeta.class);
@@ -177,6 +178,7 @@ public void testZoneAwareAndIsolationFilterWorks() {
//if errorThresholdPercentage greater than 0, it will activate.
ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.errorThresholdPercentage",
"20");
+
ArchaiusUtils.setProperty("servicecomb.loadbalance.isolation.minIsolationTime",
"10");
ServiceCombServer server2 = server;
loadBalancer = handler.getOrCreateLoadBalancer(invocation);
server = (ServiceCombServer) loadBalancer.chooseServer(invocation);
@@ -184,6 +186,10 @@ public void testZoneAwareAndIsolationFilterWorks() {
ServiceCombLoadBalancerStats.INSTANCE.markSuccess(server2);
loadBalancer = handler.getOrCreateLoadBalancer(invocation);
server = (ServiceCombServer) loadBalancer.chooseServer(invocation);
+ Assert.assertEquals(server.getEndpoint().getEndpoint(),
"rest://localhost:9091");
+ TimeUnit.MILLISECONDS.sleep(20);
+ loadBalancer = handler.getOrCreateLoadBalancer(invocation);
+ server = (ServiceCombServer) loadBalancer.chooseServer(invocation);
Assert.assertEquals(server.getEndpoint().getEndpoint(),
"rest://localhost:9090");
ServiceCombLoadBalancerStats.INSTANCE.markFailure(server2);
ServiceCombLoadBalancerStats.INSTANCE.markFailure(server2);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Isolation provide a way to isolate for at least a moment
> --------------------------------------------------------
>
> Key: SCB-961
> URL: https://issues.apache.org/jira/browse/SCB-961
> Project: Apache ServiceComb
> Issue Type: Improvement
> Reporter: liubao
> Assignee: liubao
> Priority: Major
>
> Scenario
> Assume provider will producer 50% error, and consumer access producer with
> very high volume. (concurrent access)
> Now if reached isolation condition, and will recover almost at once since
> it's quite likely a concurrent access will give a success invocation.
> In order to isolate for at least a minimal time in this scenario, we provide
> a configuration for this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
