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
commit 90aec9a599c94a9b8bffa1b8471b9d4981a51436 Author: GuoYL <[email protected]> AuthorDate: Fri Nov 22 15:29:32 2019 +0800 [SCB-1407] if don't have config,avoid registered callback,it may cause memory leak. --- .../servicecomb/router/cache/RouterRuleCache.java | 34 +++++++++++++++++----- .../router/constom/MicroserviceCache.java | 3 +- .../router/constom/RouterInvokeFilter.java | 26 +++++++++++++---- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java index 833955c..7409b5e 100644 --- a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java +++ b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/cache/RouterRuleCache.java @@ -21,13 +21,16 @@ import com.google.common.collect.Interners; import com.netflix.config.DynamicPropertyFactory; import com.netflix.config.DynamicStringProperty; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.apache.servicecomb.router.model.PolicyRuleItem; import org.apache.servicecomb.router.model.ServiceInfoCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; import org.yaml.snakeyaml.Yaml; /** @@ -51,6 +54,9 @@ public class RouterRuleCache { * @return */ public static boolean doInit(String targetServiceName) { + if (!isServerContainRule(targetServiceName)) { + return false; + } if (!serviceInfoCacheMap.containsKey(targetServiceName)) { synchronized (servicePool.intern(targetServiceName)) { if (serviceInfoCacheMap.containsKey(targetServiceName)) { @@ -62,23 +68,23 @@ public class RouterRuleCache { refresh(targetServiceName); DynamicStringProperty tepRuleStr = DynamicPropertyFactory.getInstance() .getStringProperty(String.format(ROUTE_RULE, targetServiceName), null); - addAllRule(targetServiceName, tepRuleStr); + addAllRule(targetServiceName, tepRuleStr.get()); }); - return addAllRule(targetServiceName, ruleStr); + return addAllRule(targetServiceName, ruleStr.get()); } } return true; } - private static boolean addAllRule(String targetServiceName, DynamicStringProperty ruleStr) { - if (ruleStr.get() == null) { + private static boolean addAllRule(String targetServiceName, String ruleStr) { + if (StringUtils.isEmpty(ruleStr)) { return false; } Yaml yaml = new Yaml(); List<PolicyRuleItem> policyRuleItemList; try { policyRuleItemList = Arrays - .asList(yaml.loadAs(ruleStr.get(), PolicyRuleItem[].class)); + .asList(yaml.loadAs(ruleStr, PolicyRuleItem[].class)); } catch (Exception e) { LOGGER.error("route management Serialization failed: {}", e.getMessage()); return false; @@ -87,8 +93,21 @@ public class RouterRuleCache { return false; } ServiceInfoCache serviceInfoCache = new ServiceInfoCache(policyRuleItemList); - if (!serviceInfoCacheMap.containsKey(targetServiceName)) { - serviceInfoCacheMap.put(targetServiceName, serviceInfoCache); + serviceInfoCacheMap.put(targetServiceName, serviceInfoCache); + return true; + } + + /** + * if a server don't have rule , avoid registered too many callback , it may cause memory leak + * + * @param targetServiceName + * @return + */ + public static boolean isServerContainRule(String targetServiceName) { + DynamicStringProperty lookFor = DynamicPropertyFactory.getInstance() + .getStringProperty(String.format(ROUTE_RULE, targetServiceName), null); + if (StringUtils.isEmpty(lookFor.get())) { + return false; } return true; } @@ -99,7 +118,6 @@ public class RouterRuleCache { public static void refresh() { serviceInfoCacheMap = new ConcurrentHashMap<>(); - servicePool = Interners.newWeakInterner(); } public static void refresh(String targetServiceName) { diff --git a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/MicroserviceCache.java b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/MicroserviceCache.java index e3332dd..ea7315b 100644 --- a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/MicroserviceCache.java +++ b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/MicroserviceCache.java @@ -35,9 +35,8 @@ public final class MicroserviceCache { } public Microservice getService(String serviceId) { - Microservice micorservice = services.computeIfAbsent(serviceId, (k) -> + return services.computeIfAbsent(serviceId, (k) -> RegistryUtils.getMicroservice(serviceId) ); - return micorservice; } } diff --git a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/RouterInvokeFilter.java b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/RouterInvokeFilter.java index 1e39cc1..b6a63f1 100644 --- a/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/RouterInvokeFilter.java +++ b/handlers/handler-router/src/main/java/org/apache/servicecomb/router/constom/RouterInvokeFilter.java @@ -30,6 +30,7 @@ import org.apache.servicecomb.core.definition.OperationMeta; import org.apache.servicecomb.foundation.common.utils.JsonUtils; import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx; import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx; +import org.apache.servicecomb.router.cache.RouterRuleCache; import org.apache.servicecomb.swagger.invocation.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,10 +50,6 @@ public class RouterInvokeFilter implements HttpServerFilter { private static List<String> allHeader = new ArrayList<>(); - /** - * if don't have rule , avoid registered too many callback - */ - private static boolean isFirstTime = true; @Override public int getOrder() { @@ -79,6 +76,12 @@ public class RouterInvokeFilter implements HttpServerFilter { @Override public Response afterReceiveRequest(Invocation invocation, HttpServletRequestEx httpServletRequestEx) { + if (!isHaveHeadersRule()) { + return null; + } + if (!RouterRuleCache.isServerContainRule(invocation.getMicroserviceName())) { + return null; + } if (loadHeaders()) { Map<String, String> headerMap = getHeaderMap(httpServletRequestEx); try { @@ -94,10 +97,9 @@ public class RouterInvokeFilter implements HttpServerFilter { * read config and get Header */ private boolean loadHeaders() { - if (!CollectionUtils.isEmpty(allHeader) && !isFirstTime) { + if (!CollectionUtils.isEmpty(allHeader)) { return true; } - isFirstTime = false; DynamicStringProperty headerStr = DynamicPropertyFactory.getInstance() .getStringProperty(SERVICECOMB_ROUTER_HEADER, null, () -> { DynamicStringProperty temHeader = DynamicPropertyFactory.getInstance() @@ -109,6 +111,18 @@ public class RouterInvokeFilter implements HttpServerFilter { return addAllHeaders(headerStr.get()); } + /** + * if don't have headers rule , avoid registered too many callback + */ + private boolean isHaveHeadersRule() { + DynamicStringProperty headerStr = DynamicPropertyFactory.getInstance() + .getStringProperty(SERVICECOMB_ROUTER_HEADER, null); + if (StringUtils.isEmpty(headerStr)) { + return false; + } + return true; + } + private boolean addAllHeaders(String str) { if (StringUtils.isEmpty(str)) { return false;
