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/incubator-servicecomb-java-chassis.git
commit b92c25060ee4335a921aa535758cfd2892e1779a Author: liubao <[email protected]> AuthorDate: Sat Aug 25 14:53:27 2018 +0800 [SBC-870] change loadbalancer to meet invocation level --- .../servicecomb/loadbalance/LoadBalancer.java | 24 ++++---- .../loadbalance/LoadBalancerCreator.java | 69 ---------------------- .../loadbalance/LoadbalanceHandler.java | 31 +++++----- .../loadbalance/TestLoadBalanceCreator.java | 50 +++++++++++----- .../loadbalance/TestLoadBalanceHandler2.java | 31 +++++++--- .../servicecomb/loadbalance/TestLoadBalancer.java | 10 ++-- .../loadbalance/TestLoadbalanceHandler.java | 30 +--------- .../loadbalance/TestSessionSticknessRule.java | 4 +- 8 files changed, 95 insertions(+), 154 deletions(-) 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 3a96b59..463ea19 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 @@ -20,7 +20,9 @@ package org.apache.servicecomb.loadbalance; import java.util.List; import org.apache.servicecomb.core.Invocation; +import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils; +import com.google.common.annotations.VisibleForTesting; import com.netflix.loadbalancer.LoadBalancerStats; /** @@ -35,25 +37,22 @@ public class LoadBalancer { private List<ServerListFilterExt> filters; - private List<ServiceCombServer> servers; - - public LoadBalancer(RuleExt rule, String microServiceName, - LoadBalancerStats stats, List<ServerListFilterExt> filters, List<ServiceCombServer> servers) { + public LoadBalancer(RuleExt rule, String microServiceName) { this.microServiceName = microServiceName; this.rule = rule; - this.lbStats = stats; - this.filters = filters; - this.servers = servers; + this.lbStats = new LoadBalancerStats(null); + // load new instances, because filters work on service information + this.filters = SPIServiceUtils.loadSortedService(ServerListFilterExt.class); this.rule.setLoadBalancer(this); this.filters.forEach((item) -> item.setLoadBalancer(this)); } public ServiceCombServer chooseServer(Invocation invocation) { - List<ServiceCombServer> temp = this.servers; + List<ServiceCombServer> servers = invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST); for (ServerListFilterExt filterExt : filters) { - temp = filterExt.getFilteredListOfServers(temp, invocation); + servers = filterExt.getFilteredListOfServers(servers, invocation); } - return rule.choose(temp, invocation); + return rule.choose(servers, invocation); } public LoadBalancerStats getLoadBalancerStats() { @@ -63,4 +62,9 @@ public class LoadBalancer { public String getMicroServiceName() { return microServiceName; } + + @VisibleForTesting + void setFilters(List<ServerListFilterExt> filters) { + this.filters = filters; + } } diff --git a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancerCreator.java b/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancerCreator.java deleted file mode 100644 index 5cb18dc..0000000 --- a/handlers/handler-loadbalance/src/main/java/org/apache/servicecomb/loadbalance/LoadBalancerCreator.java +++ /dev/null @@ -1,69 +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; - -import java.util.List; - -import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils; - -import com.google.common.annotations.VisibleForTesting; -import com.netflix.loadbalancer.LoadBalancerStats; - -/** - * Create a suitable load balancer for each invocation. - * - * Robin components work good in service level state, and we want to reuse its IRule components and other - * facilities, but they are not good for operation - * level filters, so write a custom load balance process. - * - * Load balance instance is created for each microservice(plus version rule), thus it is service level, - * it only can contains stateful information with service. e.g. LoadBalancerStats. - * - * ServerListFilter may choose available servers according to invocation information, IRule will work - * on the result of ServerListFilter, they should not contain operation level state information in instance fields. - */ -public class LoadBalancerCreator { - private RuleExt rule; - - private LoadBalancerStats lbStats; - - private List<ServerListFilterExt> filters; - - private String microServiceName; - - public LoadBalancerCreator(RuleExt rule, String microServiceName) { - this.rule = rule; - this.microServiceName = microServiceName; - this.lbStats = new LoadBalancerStats(null); - // load new instances, because filters work on service information - this.filters = SPIServiceUtils.loadSortedService(ServerListFilterExt.class); - } - - public void shutdown() { - // nothing to do now - } - - @VisibleForTesting - void setFilters(List<ServerListFilterExt> filters) { - this.filters = filters; - } - - public LoadBalancer createLoadBalancer(List<ServiceCombServer> servers) { - return new LoadBalancer(rule, microServiceName, lbStats, filters, servers); - } -} 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 06f4b10..9055672 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 @@ -58,6 +58,8 @@ import rx.Observable; * Load balance handler. */ public class LoadbalanceHandler implements Handler { + public static final String CONTEXT_KEY_SERVER_LIST = "x-context-server-list"; + // just a wrapper to make sure in retry mode to choose a different server. class RetryLoadBalancer implements ILoadBalancer { // Enough times to make sure to choose a different server in high volume. @@ -67,9 +69,7 @@ public class LoadbalanceHandler implements Handler { LoadBalancer delegate; - Invocation invocation; - - RetryLoadBalancer(LoadBalancer delegate, Invocation invocation) { + RetryLoadBalancer(LoadBalancer delegate) { this.delegate = delegate; } @@ -81,7 +81,7 @@ public class LoadbalanceHandler implements Handler { @Override public Server chooseServer(Object key) { for (int i = 0; i < COUNT; i++) { - Server s = delegate.chooseServer(invocation); + Server s = delegate.chooseServer((Invocation) key); if (s != null && !s.equals(lastServer)) { lastServer = s; break; @@ -131,7 +131,7 @@ public class LoadbalanceHandler implements Handler { private DiscoveryTree discoveryTree = new DiscoveryTree(); // key为grouping filter qualified name - private volatile Map<String, LoadBalancerCreator> loadBalancerMap = new ConcurrentHashMapEx<>(); + private volatile Map<String, LoadBalancer> loadBalancerMap = new ConcurrentHashMapEx<>(); private final Object lock = new Object(); @@ -154,6 +154,7 @@ public class LoadbalanceHandler implements Handler { } } this.strategy = strategy; + LoadBalancer loadBalancer = getOrCreateLoadBalancer(invocation); if (!Configuration.INSTANCE.isRetryEnabled(invocation.getMicroserviceName())) { @@ -164,9 +165,6 @@ public class LoadbalanceHandler implements Handler { } private void clearLoadBalancer() { - for (LoadBalancerCreator creator : loadBalancerMap.values()) { - creator.shutdown(); - } loadBalancerMap.clear(); } @@ -282,7 +280,7 @@ public class LoadbalanceHandler implements Handler { ExecutionContext<Invocation> context = new ExecutionContext<>(invocation, null, null, null); LoadBalancerCommand<Response> command = LoadBalancerCommand.<Response>builder() - .withLoadBalancer(new RetryLoadBalancer(chosenLB, invocation)) + .withLoadBalancer(new RetryLoadBalancer(chosenLB)) .withServerLocator(invocation) .withRetryHandler(ExtensionsManager.createRetryHandler(invocation.getMicroserviceName())) .withListeners(listeners) @@ -350,22 +348,23 @@ public class LoadbalanceHandler implements Handler { invocation.getAppId(), invocation.getMicroserviceName(), invocation.getMicroserviceVersionRule()); + invocation.addLocalContext(CONTEXT_KEY_SERVER_LIST, serversVersionedCache.data()); - LoadBalancerCreator loadBalancerCreator = loadBalancerMap.computeIfAbsent(serversVersionedCache.name(), name -> { - return createLoadBalancerCreator(invocation.getMicroserviceName()); - }); + LoadBalancer loadBalancer = loadBalancerMap + .computeIfAbsent(serversVersionedCache.name(), name -> { + return createLoadBalancer(invocation.getMicroserviceName()); + }); // Nothing to do just help users to deal with incompatible changes. setTransactionControlFilter(invocation.getMicroserviceName()); loadServerListFilters(); - return loadBalancerCreator.createLoadBalancer(serversVersionedCache.data()); + return loadBalancer; } - private LoadBalancerCreator createLoadBalancerCreator(String microserviceName) { + private LoadBalancer createLoadBalancer(String microserviceName) { RuleExt rule = ExtensionsManager.createLoadBalancerRule(microserviceName); - LoadBalancerCreator creator = new LoadBalancerCreator(rule, microserviceName); - return creator; + return new LoadBalancer(rule, microserviceName); } private void loadServerListFilters() { diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceCreator.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceCreator.java index 4c5373a..e0019b7 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceCreator.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestLoadBalanceCreator.java @@ -33,7 +33,6 @@ import com.netflix.loadbalancer.Server; import mockit.Deencapsulation; import mockit.Expectations; import mockit.Injectable; -import mockit.Mocked; public class TestLoadBalanceCreator { @Test @@ -54,7 +53,7 @@ public class TestLoadBalanceCreator { servers.add(server); servers.add(server2); - LoadBalancerCreator lbCreator = new LoadBalancerCreator(rule, "test"); + LoadBalancer lb = new LoadBalancer(rule, "test"); List<ServerListFilterExt> filters = new ArrayList<>(); @@ -72,9 +71,14 @@ public class TestLoadBalanceCreator { return filteredServers; } }); - lbCreator.setFilters(filters); + lb.setFilters(filters); - LoadBalancer lb = lbCreator.createLoadBalancer(servers); + new Expectations() { + { + invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST); + result = servers; + } + }; Server s = lb.chooseServer(invocation); Assert.assertEquals(server2, s); s = lb.chooseServer(invocation); @@ -88,7 +92,7 @@ public class TestLoadBalanceCreator { @Injectable Transport transport) { // Robin components implementations require getReachableServers & getServerList have the same size, we add a test case for this. RandomRuleExt rule = new RandomRuleExt(); - LoadBalancerCreator lbCreator = new LoadBalancerCreator(rule, "service"); + LoadBalancer lb = new LoadBalancer(rule, "service"); List<ServiceCombServer> servers = new ArrayList<>(); Endpoint host1 = new Endpoint(transport, "host1"); @@ -119,8 +123,13 @@ public class TestLoadBalanceCreator { return filteredServers; } }); - lbCreator.setFilters(filters); - LoadBalancer lb = lbCreator.createLoadBalancer(servers); + lb.setFilters(filters); + new Expectations() { + { + invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST); + result = servers; + } + }; Server s = lb.chooseServer(invocation); Assert.assertEquals(server2, s); s = lb.chooseServer(invocation); @@ -130,11 +139,11 @@ public class TestLoadBalanceCreator { } @Test - public void testLoadBalanceWithWeightedResponseTimeRuleAndFilter(@Injectable Endpoint endpoint1, @Injectable Endpoint endpoint2, @Injectable Invocation invocation) { + public void testLoadBalanceWithWeightedResponseTimeRuleAndFilter(@Injectable Endpoint endpoint1, + @Injectable Endpoint endpoint2, @Injectable Invocation invocation) { // Robin components implementations require getReachableServers & getServerList have the same size, we add a test case for this. WeightedResponseTimeRuleExt rule = new WeightedResponseTimeRuleExt(); - LoadBalancerCreator lbCreator = new LoadBalancerCreator(rule, "service"); - + LoadBalancer lb = new LoadBalancer(rule, "service"); List<ServiceCombServer> servers = new ArrayList<>(); MicroserviceInstance instance1 = new MicroserviceInstance(); @@ -170,8 +179,13 @@ public class TestLoadBalanceCreator { return filteredServers; } }); - lbCreator.setFilters(filters); - LoadBalancer lb = lbCreator.createLoadBalancer(servers); + lb.setFilters(filters); + new Expectations() { + { + invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST); + result = servers; + } + }; Server s = lb.chooseServer(invocation); Assert.assertEquals(server2, s); s = lb.chooseServer(invocation); @@ -184,7 +198,7 @@ public class TestLoadBalanceCreator { public void testLoadBalanceWithSessionSticknessRule(@Injectable Invocation invocation, @Injectable Transport transport) { SessionStickinessRule rule = new SessionStickinessRule(); - LoadBalancerCreator lbCreator = new LoadBalancerCreator(rule, "service"); + LoadBalancer lb = new LoadBalancer(rule, "service"); List<ServiceCombServer> servers = new ArrayList<>(); Endpoint host1 = new Endpoint(transport, "host1"); @@ -200,8 +214,14 @@ public class TestLoadBalanceCreator { servers.add(server); servers.add(server2); - lbCreator.setFilters(new ArrayList<>()); - LoadBalancer lb = lbCreator.createLoadBalancer(servers); + lb.setFilters(new ArrayList<>()); + new Expectations() { + { + invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST); + result = servers; + } + }; + Server s = lb.chooseServer(invocation); Assert.assertEquals(server, s); s = lb.chooseServer(invocation); 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 906529f..3dbfa32 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,6 +17,8 @@ package org.apache.servicecomb.loadbalance; +import static org.mockito.Mockito.when; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -25,6 +27,10 @@ import java.util.Map; import org.apache.servicecomb.core.CseContext; import org.apache.servicecomb.core.Invocation; import org.apache.servicecomb.core.Transport; +import org.apache.servicecomb.core.definition.MicroserviceMeta; +import org.apache.servicecomb.core.definition.OperationMeta; +import org.apache.servicecomb.core.definition.SchemaMeta; +import org.apache.servicecomb.core.provider.consumer.ReferenceConfig; import org.apache.servicecomb.core.transport.TransportManager; import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils; import org.apache.servicecomb.serviceregistry.RegistryUtils; @@ -58,7 +64,18 @@ public class TestLoadBalanceHandler2 { @Test public void testZoneAwareAndIsolationFilterWorks() { - Invocation invocation = Mockito.mock(Invocation.class); + ReferenceConfig referenceConfig = Mockito.mock(ReferenceConfig.class); + OperationMeta operationMeta = Mockito.mock(OperationMeta.class); + SchemaMeta schemaMeta = Mockito.mock(SchemaMeta.class); + when(operationMeta.getSchemaMeta()).thenReturn(schemaMeta); + MicroserviceMeta microserviceMeta = Mockito.mock(MicroserviceMeta.class); + when(schemaMeta.getMicroserviceMeta()).thenReturn(microserviceMeta); + when(schemaMeta.getMicroserviceName()).thenReturn("testMicroserviceName"); + when(microserviceMeta.getAppId()).thenReturn("testApp"); + when(referenceConfig.getVersionRule()).thenReturn("0.0.0+"); + when(referenceConfig.getTransport()).thenReturn("rest"); + Invocation invocation = new Invocation(referenceConfig, operationMeta, new Object[0]); + InstanceCacheManager instanceCacheManager = Mockito.mock(InstanceCacheManager.class); ServiceRegistry serviceRegistry = Mockito.mock(ServiceRegistry.class); TransportManager transportManager = Mockito.mock(TransportManager.class); @@ -112,15 +129,11 @@ public class TestLoadBalanceHandler2 { RegistryUtils.setServiceRegistry(serviceRegistry); - Mockito.when(invocation.getAppId()).thenReturn("testApp"); - Mockito.when(invocation.getMicroserviceName()).thenReturn("testMicroserviceName"); - Mockito.when(invocation.getMicroserviceVersionRule()).thenReturn("0.0.0+"); - Mockito.when(invocation.getConfigTransportName()).thenReturn("rest"); - Mockito.when(serviceRegistry.getMicroserviceInstance()).thenReturn(myself); - Mockito.when(serviceRegistry.getInstanceCacheManager()).thenReturn(instanceCacheManager); - Mockito.when(instanceCacheManager.getOrCreateVersionedCache("testApp", "testMicroserviceName", "0.0.0+")) + when(serviceRegistry.getMicroserviceInstance()).thenReturn(myself); + when(serviceRegistry.getInstanceCacheManager()).thenReturn(instanceCacheManager); + when(instanceCacheManager.getOrCreateVersionedCache("testApp", "testMicroserviceName", "0.0.0+")) .thenReturn(parent); - Mockito.when(transportManager.findTransport("rest")).thenReturn(transport); + when(transportManager.findTransport("rest")).thenReturn(transport); LoadbalanceHandler handler = null; LoadBalancer loadBalancer = null; 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 7b26368..761c23c 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 @@ -17,6 +17,8 @@ package org.apache.servicecomb.loadbalance; +import static org.mockito.Mockito.when; + import java.util.ArrayList; import java.util.List; @@ -34,14 +36,14 @@ public class TestLoadBalancer { ServiceCombServer server = Mockito.mock(ServiceCombServer.class); Invocation invocation = Mockito.mock(Invocation.class); newServers.add(server); - List<ServerListFilterExt> filterExts = new ArrayList<>(); - LoadBalancer loadBalancer = new LoadBalancer(rule, "test", null, filterExts, newServers); + when(invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST)).thenReturn(newServers); + LoadBalancer loadBalancer = new LoadBalancer(rule, "test"); loadBalancer.chooseServer(invocation); - Mockito.when(rule.choose(newServers, invocation)).thenReturn(server); + when(rule.choose(newServers, invocation)).thenReturn(server); Assert.assertEquals(server, loadBalancer.chooseServer(invocation)); - Assert.assertEquals(null, loadBalancer.getLoadBalancerStats()); + Assert.assertNotNull(loadBalancer.getLoadBalancerStats()); Assert.assertEquals("test", loadBalancer.getMicroServiceName()); } } 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 468e482..eedf60c 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 @@ -74,7 +74,7 @@ public class TestLoadbalanceHandler { LoadbalanceHandler handler; - Map<String, LoadBalancerCreator> loadBalancerMap; + Map<String, LoadBalancer> loadBalancerMap; @Injectable Invocation invocation; @@ -154,34 +154,6 @@ public class TestLoadbalanceHandler { } @Test - public void getOrCreateLoadBalancer() throws Exception { - LoadbalanceHandler handler = new LoadbalanceHandler(); - - MicroserviceInstance instance = new MicroserviceInstance(); - instance.setInstanceId("id"); - instance.getEndpoints().add("rest://localhost:8080"); - - Map<String, MicroserviceInstance> instanceMap = new HashMap<>(); - instanceMap.put(instance.getInstanceId(), instance); - - VersionedCache instanceVersionedCache = - new VersionedCache().autoCacheVersion().name("instanceCache").data(instanceMap); - - new Expectations() { - { - invocation.getConfigTransportName(); - result = "rest"; - instanceCacheManager.getOrCreateVersionedCache(anyString, anyString, anyString); - result = instanceVersionedCache; - } - }; - - LoadBalancer lb = handler.getOrCreateLoadBalancer(invocation); - - Assert.assertEquals("[rest://localhost:8080]", Deencapsulation.getField(lb, "servers").toString()); - } - - @Test public void send_noEndPoint(@Injectable LoadBalancer loadBalancer) { new Expectations(loadBalancer) { { diff --git a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestSessionSticknessRule.java b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestSessionSticknessRule.java index dd2319e..303735c 100644 --- a/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestSessionSticknessRule.java +++ b/handlers/handler-loadbalance/src/test/java/org/apache/servicecomb/loadbalance/TestSessionSticknessRule.java @@ -255,8 +255,8 @@ public class TestSessionSticknessRule { mockedServer.setReadyToServe(true); mockedServer.setId("mockedServer"); List<ServiceCombServer> allServers = Arrays.asList(mockedServer); - LoadBalancer lb = new LoadBalancer(rule, "mockedServer", null, new ArrayList<>(), allServers); - + LoadBalancer lb = new LoadBalancer(rule, "mockedServer"); + when(invocation.getLocalContext(LoadbalanceHandler.CONTEXT_KEY_SERVER_LIST)).thenReturn(allServers); rule.setLoadBalancer(lb); ServiceCombServer server = new ServiceCombServer(transport, new CacheEndpoint("rest:127.0.0.1:8890", instance1)); Deencapsulation.setField(rule, "lastServer", server);
