From f2a10f578f887251f6faa62045f258607a4bcbd0 Mon Sep 17 00:00:00 2001 From: chengyouling Date: Wed, 19 Jul 2023 09:13:22 +0800 Subject: [PATCH 1/2] [#3860] Optimize gray routing rule algorithm --- .../distribute/AbstractRouterDistributor.java | 65 ++++--------- .../router/model/PolicyRuleItem.java | 12 +-- .../RouterDistributorFileWeightLessTest.java | 94 +++++++++++++++++++ .../src/test/resources/application.yaml | 7 ++ 4 files changed, 123 insertions(+), 55 deletions(-) create mode 100644 governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileWeightLessTest.java diff --git a/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java b/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java index 4de2e6a720..f2c691d257 100644 --- a/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java +++ b/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java @@ -21,13 +21,11 @@ import java.util.List; import java.util.Map; import java.util.function.Function; -import java.util.stream.Collectors; import org.apache.servicecomb.router.cache.RouterRuleCache; import org.apache.servicecomb.router.model.PolicyRuleItem; import org.apache.servicecomb.router.model.RouteItem; import org.apache.servicecomb.router.model.TagItem; -import org.apache.servicecomb.router.util.VersionCompareUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -56,25 +54,32 @@ protected AbstractRouterDistributor() { @Override public List distribute(String targetServiceName, List list, PolicyRuleItem invokeRule) { - //init LatestVersion - initLatestVersion(targetServiceName, list); - invokeRule.check( - routerRuleCache.getServiceInfoCacheMap().get(targetServiceName).getLatestVersionTag()); + invokeRule.check(); + + // unSetTags instance list + List unSetTagInstances = new ArrayList<>(); // get tag list - Map> versionServerMap = getDistributList(targetServiceName, list, invokeRule); + Map> versionServerMap = getDistributList(targetServiceName, list, invokeRule, unSetTagInstances); if (CollectionUtils.isEmpty(versionServerMap)) { LOGGER.debug("route management can not match any rule and route the latest version"); - return getLatestVersionList(list, targetServiceName); + // rule note matched instance babel, all instance return, select instance for load balancing later + return list; } + // weight calculation to obtain the next tags instance TagItem targetTag = getFiltedServerTagItem(invokeRule, targetServiceName); - if (versionServerMap.containsKey(targetTag)) { + if (targetTag != null && versionServerMap.containsKey(targetTag)) { return versionServerMap.get(targetTag); } - return getLatestVersionList(list, targetServiceName); + + // has weightLess situation + if (invokeRule.isWeightLess()) { + return unSetTagInstances; + } + return list; } @Override @@ -98,10 +103,7 @@ public TagItem getFiltedServerTagItem(PolicyRuleItem rule, String targetServiceN * the method getProperties() contains other field that we don't need. */ private Map> getDistributList(String serviceName, - List list, - PolicyRuleItem invokeRule) { - String latestV = routerRuleCache.getServiceInfoCacheMap().get(serviceName).getLatestVersionTag() - .getVersion(); + List list, PolicyRuleItem invokeRule, List unSetTagInstances) { Map> versionServerMap = new HashMap<>(); for (INSTANCE instance : list) { //get server @@ -110,6 +112,7 @@ private Map> getDistributList(String serviceName, TagItem tagItem = new TagItem(getVersion.apply(instance), getProperties.apply(instance)); TagItem targetTag = null; int maxMatch = 0; + // obtain the rule with the most parameter matches for (RouteItem entry : invokeRule.getRoute()) { if (entry.getTagitem() == null){ continue; @@ -120,45 +123,17 @@ private Map> getDistributList(String serviceName, targetTag = entry.getTagitem(); } } - if (invokeRule.isWeightLess() && getVersion.apply(instance).equals(latestV)) { - TagItem latestVTag = invokeRule.getRoute().get(invokeRule.getRoute().size() - 1) - .getTagitem(); - if (!versionServerMap.containsKey(latestVTag)) { - versionServerMap.put(latestVTag, new ArrayList<>()); - } - versionServerMap.get(latestVTag).add(instance); - } if (targetTag != null) { if (!versionServerMap.containsKey(targetTag)) { versionServerMap.put(targetTag, new ArrayList<>()); } versionServerMap.get(targetTag).add(instance); + } else { + // not matched, placed in the unset tag instances collection + unSetTagInstances.add(instance); } } } return versionServerMap; } - - - public void initLatestVersion(String serviceName, List list) { - String latestVersion = null; - for (INSTANCE instance : list) { - if (getServerName.apply(instance).equals(serviceName)) { - if (latestVersion == null || VersionCompareUtil - .compareVersion(latestVersion, getVersion.apply(instance)) == -1) { - latestVersion = getVersion.apply(instance); - } - } - } - TagItem tagitem = new TagItem(latestVersion); - routerRuleCache.getServiceInfoCacheMap().get(serviceName).setLatestVersionTag(tagitem); - } - - public List getLatestVersionList(List list, String targetServiceName) { - String latestV = routerRuleCache.getServiceInfoCacheMap().get(targetServiceName) - .getLatestVersionTag().getVersion(); - return list.stream().filter(instance -> - getVersion.apply(instance).equals(latestV) - ).collect(Collectors.toList()); - } } diff --git a/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java b/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java index edf950505e..1006abfec1 100644 --- a/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java +++ b/governance/src/main/java/org/apache/servicecomb/router/model/PolicyRuleItem.java @@ -48,16 +48,11 @@ public PolicyRuleItem() { /** * if weight is less than 100, fill with minimum version * - * @param latestVersionTag */ - public void check(TagItem latestVersionTag) { + public void check() { if (CollectionUtils.isEmpty(route)) { throw new RouterIllegalParamException("canary rule list can not be null"); } - if (route.size() == 1) { - route.get(0).setWeight(100); - return; - } int sum = 0; for (RouteItem item : route) { if (item.getWeight() == null) { @@ -68,11 +63,8 @@ public void check(TagItem latestVersionTag) { if (sum > 100) { LOGGER.warn("canary rule weight sum is more than 100"); } else if (sum < 100) { - if (latestVersionTag == null) { - LOGGER.warn("canary has some error when set default latestVersion"); - } weightLess = true; - route.add(new RouteItem(100 - sum, latestVersionTag)); + route.add(new RouteItem(100 - sum, null)); } } diff --git a/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileWeightLessTest.java b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileWeightLessTest.java new file mode 100644 index 0000000000..763e2eeca0 --- /dev/null +++ b/governance/src/test/java/org/apache/servicecomb/router/RouterDistributorFileWeightLessTest.java @@ -0,0 +1,94 @@ +/* + * 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.router; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import org.apache.servicecomb.router.distribute.RouterDistributor; +import org.junit.Test; +import org.junit.jupiter.api.Assertions; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +@RunWith(SpringRunner.class) +@ContextConfiguration(locations = "classpath:META-INF/spring/*.xml", initializers = ConfigDataApplicationContextInitializer.class) +public class RouterDistributorFileWeightLessTest { + private static final String TARGET_SERVICE_NAME = "test_server2"; + + private RouterFilter routerFilter; + + private RouterDistributor routerDistributor; + + @Autowired + public void setRouterFilter(RouterFilter routerFilter) { + this.routerFilter = routerFilter; + } + + @Autowired + public void setRouterDistributor(RouterDistributor routerDistributor) { + this.routerDistributor = routerDistributor; + } + + @Test + public void testDistribute() { + List list = initServiceList(); + HashMap header = new HashMap<>(); + List listOfServers = routerFilter + .getFilteredListOfServers(list, TARGET_SERVICE_NAME, header, routerDistributor); + Assertions.assertNotNull(listOfServers); + for (ServiceIns server : listOfServers) { + Assertions.assertEquals(TARGET_SERVICE_NAME, server.getServerName()); + } + int serverNum1 = 0; + int unSetTagNum = 0; + + for (int i = 0; i < 10; i++) { + List serverList = routerFilter + .getFilteredListOfServers(list, TARGET_SERVICE_NAME, header, routerDistributor); + for (ServiceIns serviceIns : serverList) { + if ("01".equals(serviceIns.getId())) { + serverNum1++; + } else if ("02".equals(serviceIns.getId())) { + unSetTagNum++; + } + } + } + boolean flag = false; + if (Math.round(unSetTagNum * 1.0 / serverNum1) == 4) { + flag = true; + } + Assertions.assertTrue(flag); + } + + List initServiceList() { + ServiceIns serviceIns1 = new ServiceIns("01", "test_server2"); + ServiceIns serviceIns2 = new ServiceIns("02", "test_server2"); + serviceIns1.setVersion("1.0"); + serviceIns2.setVersion("2.0"); + serviceIns1.addTags("x-group", "red"); + serviceIns2.addTags("x-group", "green"); + List list = new ArrayList<>(); + list.add(serviceIns1); + list.add(serviceIns2); + return list; + } +} diff --git a/governance/src/test/resources/application.yaml b/governance/src/test/resources/application.yaml index e6ba6c73b5..95eb39f61c 100644 --- a/governance/src/test/resources/application.yaml +++ b/governance/src/test/resources/application.yaml @@ -235,3 +235,10 @@ servicecomb: - weight: 80 tags: x-group: green + + test_server2: | # 服务名 + - precedence: 1 + route: + - weight: 20 + tags: + x-group: red \ No newline at end of file From f3278f06a96078d37bac8f7a88902dcd5227a325 Mon Sep 17 00:00:00 2001 From: chengyouling Date: Wed, 19 Jul 2023 14:44:26 +0800 Subject: [PATCH 2/2] supplement settings weights less than 100%, but contain all provider instances --- .../router/distribute/AbstractRouterDistributor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java b/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java index f2c691d257..731d05d976 100644 --- a/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java +++ b/governance/src/main/java/org/apache/servicecomb/router/distribute/AbstractRouterDistributor.java @@ -76,7 +76,7 @@ public List distribute(String targetServiceName, List list, } // has weightLess situation - if (invokeRule.isWeightLess()) { + if (invokeRule.isWeightLess() && unSetTagInstances.size() > 0) { return unSetTagInstances; } return list;