You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2020/05/26 09:43:34 UTC

[servicecomb-java-chassis] 01/02: [SCB-1930]When MicroserivceVersions.setInstances continues fail will cause OOM

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 0c41408c5cf1f9989cff1cdb421834c6dbef5d6f
Author: liubao <bi...@qq.com>
AuthorDate: Mon May 25 15:32:31 2020 +0800

    [SCB-1930]When MicroserivceVersions.setInstances continues fail will cause OOM
---
 .../config/inject/ConfigObjectFactory.java         |  3 +-
 .../config/priority/PriorityProperty.java          | 14 +++++----
 .../config/priority/PriorityPropertyManager.java   | 34 +++++++++++++---------
 .../inspector/internal/InspectorImpl.java          |  3 +-
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/inject/ConfigObjectFactory.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/inject/ConfigObjectFactory.java
index 8dee778..1c73d84 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/inject/ConfigObjectFactory.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/inject/ConfigObjectFactory.java
@@ -25,7 +25,6 @@ import org.apache.servicecomb.config.priority.PriorityProperty;
 import org.apache.servicecomb.config.priority.PriorityPropertyManager;
 import org.apache.servicecomb.foundation.common.utils.JsonUtils;
 import org.apache.servicecomb.foundation.common.utils.LambdaMetafactoryUtils;
-import org.apache.servicecomb.foundation.common.utils.bean.Setter;
 import org.apache.servicecomb.foundation.common.utils.bean.SetterWrapper;
 
 import com.fasterxml.jackson.databind.BeanDescription;
@@ -104,7 +103,7 @@ public class ConfigObjectFactory {
           new SetterWrapper(LambdaMetafactoryUtils.createSetter(propertyDefinition.getSetter().getAnnotated()));
 
       PriorityProperty<?> priorityProperty = createPriorityProperty(propertyDefinition.getField().getAnnotated());
-      priorityProperty.setCallback(value -> setter.set(instance, value));
+      priorityProperty.setCallback((value, target) -> setter.set(target, value), instance);
       priorityProperties.add(priorityProperty);
     }
   }
diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityProperty.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityProperty.java
index 53cc9cc..a07f8c3 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityProperty.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityProperty.java
@@ -19,7 +19,7 @@ package org.apache.servicecomb.config.priority;
 import java.lang.reflect.Type;
 import java.util.Arrays;
 import java.util.Objects;
-import java.util.function.Consumer;
+import java.util.function.BiConsumer;
 import java.util.function.Function;
 
 import org.slf4j.Logger;
@@ -53,7 +53,7 @@ public class PriorityProperty<T> {
 
   private Function<DynamicProperty, T> internalValueReader;
 
-  private Consumer<T> callback = v -> {
+  private BiConsumer<T, Object> callback = (t, u) -> {
   };
 
   @SuppressWarnings("unchecked")
@@ -144,6 +144,10 @@ public class PriorityProperty<T> {
   }
 
   synchronized void updateFinalValue(boolean init) {
+    updateFinalValue(init, this);
+  }
+
+  synchronized void updateFinalValue(boolean init, Object target) {
     T lastValue = finalValue;
 
     String effectiveKey = "default value";
@@ -171,15 +175,15 @@ public class PriorityProperty<T> {
           joinedPriorityKeys, finalValue, value, effectiveKey);
     }
     finalValue = value;
-    callback.accept(finalValue);
+    callback.accept(finalValue, target);
   }
 
   public T getValue() {
     return finalValue;
   }
 
-  public void setCallback(Consumer<T> callback) {
+  public void setCallback(BiConsumer<T, Object> callback, Object target) {
     this.callback = callback;
-    callback.accept(finalValue);
+    callback.accept(finalValue, target);
   }
 }
diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
index 2bbceb2..a7d8544 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
@@ -23,25 +23,29 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.WeakHashMap;
 
 import org.apache.commons.configuration.event.ConfigurationEvent;
 import org.apache.commons.configuration.event.ConfigurationListener;
 import org.apache.servicecomb.config.inject.ConfigObjectFactory;
 import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.netflix.config.ConfigurationManager;
 import com.netflix.config.DynamicPropertyFactory;
 
 public class PriorityPropertyManager {
   private ConfigurationListener configurationListener = this::configurationListener;
 
-  private Map<PriorityProperty<?>, PriorityProperty<?>> priorityPropertyMap = new ConcurrentHashMapEx<>();
+  private Map<PriorityProperty<?>, Boolean> priorityPropertyMap =
+      Collections.synchronizedMap(new WeakHashMap<>());
 
-  private Map<Object, List<PriorityProperty<?>>> configObjectMap = new ConcurrentHashMapEx<>();
+  private Map<Object, List<PriorityProperty<?>>> configObjectMap =
+      Collections.synchronizedMap(new WeakHashMap<>());
 
   // will be reset to null after register or unregister
   // and build when configuration changed
-  private Map<String, List<PriorityProperty<?>>> keyCache;
+  private Map<Object, Map<String, List<PriorityProperty<?>>>> keyCache;
 
   public PriorityPropertyManager() {
     // make sure create a DynamicPropertyFactory instance
@@ -62,30 +66,33 @@ public class PriorityPropertyManager {
 
     if (keyCache == null) {
       keyCache = new ConcurrentHashMapEx<>();
-      updateCache(priorityPropertyMap.values());
-      configObjectMap.values().stream().forEach(this::updateCache);
+      updateCache(new Object(), priorityPropertyMap.keySet());
+      configObjectMap.forEach((k, v) -> updateCache(k, v));
     }
 
     if (event.getPropertyName() != null) {
-      keyCache.getOrDefault(event.getPropertyName(), Collections.emptyList()).stream()
-          .forEach(p -> p.updateFinalValue(false));
+      keyCache.forEach((k, v) -> v.getOrDefault(event.getPropertyName(), Collections.emptyList()).stream()
+          .forEach(p -> p.updateFinalValue(false, k)));
       return;
     }
 
     // event like add configuration source, need to make a global refresh
-    keyCache.values().stream().flatMap(Collection::stream).forEach(p -> p.updateFinalValue(false));
+    keyCache.forEach(
+        (k, v) -> v.values().stream().flatMap(Collection::stream).forEach(
+            p -> p.updateFinalValue(false, k)));
   }
 
-  private void updateCache(Collection<PriorityProperty<?>> properties) {
+  private void updateCache(Object target, Collection<PriorityProperty<?>> properties) {
+    Map<String, List<PriorityProperty<?>>> targetMap = keyCache.computeIfAbsent(target, k -> new HashMap<>());
     for (PriorityProperty<?> priorityProperty : properties) {
       for (String key : priorityProperty.getPriorityKeys()) {
-        keyCache.computeIfAbsent(key, k -> new ArrayList<>()).add(priorityProperty);
+        targetMap.computeIfAbsent(key, k -> new ArrayList<>()).add(priorityProperty);
       }
-      priorityProperty.updateFinalValue(false);
+      priorityProperty.updateFinalValue(false, target);
     }
   }
 
-  public Map<PriorityProperty<?>, PriorityProperty<?>> getPriorityPropertyMap() {
+  public Map<PriorityProperty<?>, Boolean> getPriorityPropertyMap() {
     return priorityPropertyMap;
   }
 
@@ -94,7 +101,7 @@ public class PriorityPropertyManager {
   }
 
   private synchronized void registerPriorityProperty(PriorityProperty<?> property) {
-    priorityPropertyMap.put(property, property);
+    priorityPropertyMap.put(property, Boolean.TRUE);
     keyCache = null;
   }
 
@@ -138,6 +145,7 @@ public class PriorityPropertyManager {
     return new PriorityProperty<>(cls, invalidValue, defaultValue, priorityKeys);
   }
 
+  @VisibleForTesting
   public <T> PriorityProperty<T> createPriorityProperty(Type cls, T invalidValue, T defaultValue,
       String... priorityKeys) {
     PriorityProperty<T> priorityProperty = new PriorityProperty<>(cls, invalidValue, defaultValue, priorityKeys);
diff --git a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
index e86ad2a..6e87be2 100644
--- a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
+++ b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
@@ -314,7 +314,8 @@ public class InspectorImpl {
     priorityPropertyManager.getConfigObjectMap().values().stream()
         .flatMap(Collection::stream)
         .forEach(p -> views.add(createPriorityPropertyView(p)));
-    priorityPropertyManager.getPriorityPropertyMap().values().forEach(p -> views.add(createPriorityPropertyView(p)));
+    priorityPropertyManager.getPriorityPropertyMap().keySet().forEach(p ->
+        views.add(createPriorityPropertyView(p)));
     return views;
   }