[
https://issues.apache.org/jira/browse/SCB-416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16408925#comment-16408925
]
ASF GitHub Bot commented on SCB-416:
------------------------------------
liubao68 closed pull request #613: [SCB-416]For load balance rule
configurations, we need provider service level configuration
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/613
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 9e40cd70c..3a2043bf9 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
@@ -101,10 +101,10 @@ public String getRuleStrategyName(String microservice) {
PROP_ROOT + PROP_RULE_STRATEGY_NAME);
}
- public int getSessionTimeoutInSeconds() {
+ public int getSessionTimeoutInSeconds(String microservice) {
final int defaultValue = 30;
- // do not support MicroService level now
String p = getStringProperty("30",
+ PROP_ROOT + microservice + "." + SESSION_TIMEOUT_IN_SECONDS,
PROP_ROOT + SESSION_TIMEOUT_IN_SECONDS);
try {
return Integer.parseInt(p); // can be negative
@@ -113,10 +113,10 @@ public int getSessionTimeoutInSeconds() {
}
}
- public int getSuccessiveFailedTimes() {
+ public int getSuccessiveFailedTimes(String microservice) {
final int defaultValue = 5;
- // do not support MicroService level now
String p = getStringProperty("5",
+ PROP_ROOT + microservice + "." + SUCCESSIVE_FAILED_TIMES,
PROP_ROOT + SUCCESSIVE_FAILED_TIMES);
try {
return Integer.parseInt(p); // can be negative
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 e108debf1..ec656918f 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
@@ -46,12 +46,15 @@
// 以filter类名为Key
private Map<String, ServerListFilterExt> filters;
- public LoadBalancer(String name, IRule rule) {
+ private String microServiceName;
+
+ public LoadBalancer(String name, IRule rule, String microServiceName) {
this.name = name;
this.rule = rule;
- this.rule.setLoadBalancer(this);
+ this.microServiceName = microServiceName;
this.lbStats = new LoadBalancerStats(null);
this.filters = new ConcurrentHashMap<>();
+ this.rule.setLoadBalancer(this);
}
public String getName() {
@@ -122,4 +125,8 @@ public void putFilter(String name, ServerListFilterExt
filter) {
public int getFilterSize() {
return filters.size();
}
+
+ public String getMicroServiceName() {
+ return microServiceName;
+ }
}
diff --git
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
index 2d9d9288e..fd22f8ec5 100644
---
a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
+++
b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadbalanceHandler.java
@@ -311,7 +311,7 @@ protected LoadBalancer getOrCreateLoadBalancer(Invocation
invocation) {
private LoadBalancer createLoadBalancer(String microserviceName, String
loadBalancerName) {
IRule rule = ExtensionsManager.createLoadBalancerRule(microserviceName);
- LoadBalancer lb = new LoadBalancer(loadBalancerName, rule);
+ LoadBalancer lb = new LoadBalancer(loadBalancerName, rule,
microserviceName);
// we can change this implementation to ExtensionsManager in the future.
loadServerListFilters(lb);
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 71c770984..5a3a2c2b1 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
@@ -51,6 +51,8 @@
private static final int MILLI_COUNT_IN_SECOND = 1000;
+ private String microserviceName;
+
public SessionStickinessRule() {
triggerRule = new RoundRobinRule();
}
@@ -92,9 +94,9 @@ private Server chooseServerErrorThresholdMet(Object key) {
}
private boolean isTimeOut() {
- return Configuration.INSTANCE.getSessionTimeoutInSeconds() > 0
+ return Configuration.INSTANCE.getSessionTimeoutInSeconds(microserviceName)
> 0
&& System.currentTimeMillis()
- - this.lastAccessedTime > ((long)
Configuration.INSTANCE.getSessionTimeoutInSeconds()
+ - this.lastAccessedTime > ((long)
Configuration.INSTANCE.getSessionTimeoutInSeconds(microserviceName)
* MILLI_COUNT_IN_SECOND);
}
@@ -105,8 +107,8 @@ private boolean isErrorThresholdMet() {
if (stats != null && stats.getServerStats() != null &&
stats.getServerStats().size() > 0) {
ServerStats serverStats = stats.getSingleServerStat(lastServer);
int successiveFaildCount =
serverStats.getSuccessiveConnectionFailureCount();
- if (Configuration.INSTANCE.getSuccessiveFailedTimes() > 0
- && successiveFaildCount >=
Configuration.INSTANCE.getSuccessiveFailedTimes()) {
+ if (Configuration.INSTANCE.getSuccessiveFailedTimes(microserviceName) > 0
+ && successiveFaildCount >=
Configuration.INSTANCE.getSuccessiveFailedTimes(microserviceName)) {
serverStats.clearSuccessiveConnectionFailureCount();
return true;
}
@@ -139,6 +141,7 @@ public Server choose(Object key) {
@Override
public void setLoadBalancer(ILoadBalancer lb) {
this.lb = lb;
+ this.microserviceName = ((LoadBalancer) lb).getMicroServiceName();
}
@Override
diff --git
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java
index 5ba504226..703315dbd 100644
---
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java
+++
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestConfiguration.java
@@ -47,20 +47,12 @@ public void testConstants() {
@Test
public void testFullConfigurationWithArgsString() {
-
- Configuration.INSTANCE.getPolicy("test");
- Configuration.INSTANCE.getRetryOnNext("test");
- Configuration.INSTANCE.getRetryOnSame("test");
- Configuration.INSTANCE.isRetryEnabled("test");
- Configuration.INSTANCE.getSuccessiveFailedTimes();
- Configuration.INSTANCE.getSessionTimeoutInSeconds();
-
assertNull(Configuration.INSTANCE.getPolicy("test"));
assertNotNull(Configuration.INSTANCE.getRetryOnNext("test"));
assertNotNull(Configuration.INSTANCE.getRetryOnSame("test"));
assertNotNull(Configuration.INSTANCE.isRetryEnabled("test"));
- assertNotNull(Configuration.INSTANCE.getSuccessiveFailedTimes());
- assertNotNull(Configuration.INSTANCE.getSessionTimeoutInSeconds());
+ assertNotNull(Configuration.INSTANCE.getSuccessiveFailedTimes("test"));
+ assertNotNull(Configuration.INSTANCE.getSessionTimeoutInSeconds("test"));
}
@Test
@@ -154,15 +146,11 @@ private String getProperty(String defaultValue, String...
keys) {
@Test
public void testGetSuccessiveFailedTimes() {
-
- Configuration.INSTANCE.getSuccessiveFailedTimes();
- assertNotNull(Configuration.INSTANCE.getSuccessiveFailedTimes());
+ assertNotNull(Configuration.INSTANCE.getSuccessiveFailedTimes("test"));
}
@Test
public void testGetSessionTimeoutInSeconds() {
-
- Configuration.INSTANCE.getSessionTimeoutInSeconds();
- assertNotNull(Configuration.INSTANCE.getSessionTimeoutInSeconds());
+ assertNotNull(Configuration.INSTANCE.getSessionTimeoutInSeconds("test"));
}
}
diff --git
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalancer.java
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalancer.java
index d24318b56..1ebce0e05 100644
---
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalancer.java
+++
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalancer.java
@@ -22,6 +22,7 @@
import java.util.ArrayList;
import java.util.List;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
import
org.apache.servicecomb.loadbalance.filter.SimpleTransactionControlFilter;
import org.apache.servicecomb.loadbalance.filter.TransactionControlFilter;
import org.junit.Assert;
@@ -32,10 +33,12 @@
import com.netflix.loadbalancer.IRule;
import com.netflix.loadbalancer.Server;
+import mockit.Deencapsulation;
+
public class TestLoadBalancer {
private IRule rule = Mockito.mock(IRule.class);
- private LoadBalancer loadBalancer = new LoadBalancer("loadBalancerName",
rule);
+ private LoadBalancer loadBalancer = new LoadBalancer("loadBalancerName",
rule, "test");
@Test
public void name() {
@@ -146,4 +149,41 @@ public void testGetAllServers() {
Mockito.when(filter.getFilteredListOfServers(servers)).thenReturn(servers);
Assert.assertEquals(servers, loadBalancer.getAllServers());
}
+
+ @Test
+ public void testLoadBalanceWithSessionSticknessRule() {
+ SessionStickinessRule rule = new SessionStickinessRule();
+ LoadBalancer lb = new LoadBalancer("lb1", rule, "service");
+ Assert.assertEquals(lb.getMicroServiceName(), "service");
+ Assert.assertEquals("service", Deencapsulation.getField(rule,
"microserviceName"));
+
+ List<Server> servers = new ArrayList<>();
+ Server server = new Server("host1", 80);
+ server.setAlive(true);
+ Server server2 = new Server("host2", 80);
+ server2.setAlive(true);
+ servers.add(server);
+ servers.add(server2);
+ lb.setServerList(servers);
+
+ Server s = lb.chooseServer("test");
+ Assert.assertEquals(server2, s);
+ s = lb.chooseServer("test");
+ Assert.assertEquals(server2, s);
+
+ long time = Deencapsulation.getField(rule, "lastAccessedTime");
+ Deencapsulation.setField(rule, "lastAccessedTime", time - 1000 * 10);
+
ArchaiusUtils.setProperty("cse.loadbalance.service.SessionStickinessRule.sessionTimeoutInSeconds",
9);
+ s = lb.chooseServer("test");
+ Assert.assertEquals(server, s);
+
+
ArchaiusUtils.setProperty("cse.loadbalance.service.SessionStickinessRule.successiveFailedTimes",
5);
+ lb.getLoadBalancerStats().incrementSuccessiveConnectionFailureCount(s);
+ lb.getLoadBalancerStats().incrementSuccessiveConnectionFailureCount(s);
+ lb.getLoadBalancerStats().incrementSuccessiveConnectionFailureCount(s);
+ lb.getLoadBalancerStats().incrementSuccessiveConnectionFailureCount(s);
+ lb.getLoadBalancerStats().incrementSuccessiveConnectionFailureCount(s);
+ s = lb.chooseServer("test");
+ Assert.assertEquals(server2, s);
+ }
}
diff --git
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadbalanceHandler.java
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadbalanceHandler.java
index 5930335fc..85c91a796 100644
---
a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadbalanceHandler.java
+++
b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadbalanceHandler.java
@@ -76,7 +76,7 @@
Map<String, LoadBalancer> loadBalancerMap =
Deencapsulation.getField(handler, "loadBalancerMap");
- private LoadBalancer loadBalancer = new LoadBalancer("loadBalancerName",
rule);
+ private LoadBalancer loadBalancer = new LoadBalancer("loadBalancerName",
rule, "test");
@Injectable
Invocation invocation;
@@ -238,12 +238,12 @@ public void testSetIsolationFilter() {
Invocation invocation = Mockito.mock(Invocation.class);
Mockito.when(invocation.getMicroserviceName()).thenReturn("test");
LoadbalanceHandler lbHandler = new LoadbalanceHandler();
- LoadBalancer myLB = new LoadBalancer("loadBalancerName", rule);
+ LoadBalancer myLB = new LoadBalancer("loadBalancerName", rule, "test");
lbHandler.setIsolationFilter(myLB, "abc");
Assert.assertEquals(1, myLB.getFilterSize());
Mockito.when(invocation.getMicroserviceName()).thenReturn("abc");
- myLB = new LoadBalancer("loadBalancerName", rule);
+ myLB = new LoadBalancer("loadBalancerName", rule, "test");
lbHandler.setIsolationFilter(myLB, "abc");
myLB.setInvocation(invocation);
----------------------------------------------------------------
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]
> For load balance rule configurations, we need provider service level
> configuration
> ----------------------------------------------------------------------------------
>
> Key: SCB-416
> URL: https://issues.apache.org/jira/browse/SCB-416
> Project: Apache ServiceComb
> Issue Type: Improvement
> Reporter: liubao
> Assignee: liubao
> Priority: Major
>
> Description:
> Now we can configure service level value to load balancer, e.g.
> cse.loadbalance.[serviceName].policy
> But we can not configure for Rule, e.g.
>
> cse.loadbalance.SessionStickinessRule.sessionTimeoutInSeconds
>
> we need to enable configurations like:
> cse.loadbalance.[serviceName].SessionStickinessRule.sessionTimeoutInSeconds
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
