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:33 UTC

[servicecomb-java-chassis] branch master updated (e413d63 -> 71fd15c)

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git.


    from e413d63  [SCB-1931] fix consumer flag in MicroserviceMeta
     new 0c41408  [SCB-1930]When MicroserivceVersions.setInstances continues fail will cause OOM
     new 71fd15c  [SCB-1930]using set for priorityproperty cache and remove unnecessary code

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../core/definition/MicroserviceVersionsMeta.java  |   5 -
 .../core/definition/ServiceRegistryListener.java   |   5 +-
 .../config/inject/ConfigObjectFactory.java         |   3 +-
 .../config/priority/PriorityProperty.java          |  14 ++-
 .../config/priority/PriorityPropertyManager.java   |  49 ++++-----
 .../config/inject/TestConfigObjectFactory.java     |   8 --
 .../config/priority/TestPriorityProperty.java      |  14 ---
 .../config/priority/TestPriorityPropertyBase.java  |  10 --
 .../priority/TestPriorityPropertyManager.java      | 109 +++++++++++++++++++++
 .../inspector/internal/InspectorBootListener.java  |  13 +--
 .../inspector/internal/InspectorImpl.java          |   3 +-
 .../internal/TestInspectorBootListener.java        |   1 -
 .../inspector/internal/TestInspectorImpl.java      |   6 --
 13 files changed, 145 insertions(+), 95 deletions(-)
 create mode 100644 foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java


[servicecomb-java-chassis] 02/02: [SCB-1930]using set for priorityproperty cache and remove unnecessary code

Posted by li...@apache.org.
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 71fd15c87858e88532f26642d739caef77ad6d64
Author: liubao <bi...@qq.com>
AuthorDate: Tue May 26 10:40:52 2020 +0800

    [SCB-1930]using set for priorityproperty cache and remove unnecessary code
---
 .../core/definition/MicroserviceVersionsMeta.java  |   5 -
 .../core/definition/ServiceRegistryListener.java   |   5 +-
 .../config/priority/PriorityPropertyManager.java   |  29 ++----
 .../config/inject/TestConfigObjectFactory.java     |   8 --
 .../config/priority/TestPriorityProperty.java      |  14 ---
 .../config/priority/TestPriorityPropertyBase.java  |  10 --
 .../priority/TestPriorityPropertyManager.java      | 109 +++++++++++++++++++++
 .../inspector/internal/InspectorBootListener.java  |  13 +--
 .../inspector/internal/InspectorImpl.java          |   2 +-
 .../internal/TestInspectorBootListener.java        |   1 -
 .../inspector/internal/TestInspectorImpl.java      |   6 --
 11 files changed, 120 insertions(+), 82 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java b/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
index 307b51a..8252d2f 100644
--- a/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
+++ b/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
@@ -35,11 +35,6 @@ public class MicroserviceVersionsMeta {
         .createConfigObject(MicroserviceConfig.class, "service", microserviceName);
   }
 
-  public void destroy() {
-    scbEngine.getPriorityPropertyManager().unregisterConfigObject(microserviceConfig);
-    configs.values().stream()
-        .forEach(scbEngine.getPriorityPropertyManager()::unregisterConfigObject);
-  }
 
   public MicroserviceConfig getMicroserviceConfig() {
     return microserviceConfig;
diff --git a/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java b/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
index 0931864..5077234 100644
--- a/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
+++ b/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
@@ -64,10 +64,7 @@ public class ServiceRegistryListener {
   @SubscriberOrder(-1000)
   @Subscribe
   public void onDestroyMicroservice(DestroyMicroserviceEvent event) {
-    MicroserviceVersions microserviceVersions = event.getMicroserviceVersions();
-    MicroserviceVersionsMeta microserviceVersionsMeta = microserviceVersions.getVendorExtensions()
-        .get(CORE_MICROSERVICE_VERSIONS_META);
-    microserviceVersionsMeta.destroy();
+
   }
 
   @EnableExceptionPropagation
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 a7d8544..dd84f5a 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,6 +23,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.WeakHashMap;
 
 import org.apache.commons.configuration.event.ConfigurationEvent;
@@ -30,15 +31,14 @@ 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<?>, Boolean> priorityPropertyMap =
-      Collections.synchronizedMap(new WeakHashMap<>());
+  private Set<PriorityProperty<?>> priorityPropertySet =
+      Collections.newSetFromMap(Collections.synchronizedMap(new WeakHashMap<>()));
 
   private Map<Object, List<PriorityProperty<?>>> configObjectMap =
       Collections.synchronizedMap(new WeakHashMap<>());
@@ -66,7 +66,7 @@ public class PriorityPropertyManager {
 
     if (keyCache == null) {
       keyCache = new ConcurrentHashMapEx<>();
-      updateCache(new Object(), priorityPropertyMap.keySet());
+      updateCache(new Object(), priorityPropertySet);
       configObjectMap.forEach((k, v) -> updateCache(k, v));
     }
 
@@ -92,8 +92,8 @@ public class PriorityPropertyManager {
     }
   }
 
-  public Map<PriorityProperty<?>, Boolean> getPriorityPropertyMap() {
-    return priorityPropertyMap;
+  public Set<PriorityProperty<?>> getPriorityPropertySet() {
+    return priorityPropertySet;
   }
 
   public Map<Object, List<PriorityProperty<?>>> getConfigObjectMap() {
@@ -101,7 +101,7 @@ public class PriorityPropertyManager {
   }
 
   private synchronized void registerPriorityProperty(PriorityProperty<?> property) {
-    priorityPropertyMap.put(property, Boolean.TRUE);
+    priorityPropertySet.add(property);
     keyCache = null;
   }
 
@@ -110,20 +110,6 @@ public class PriorityPropertyManager {
     keyCache = null;
   }
 
-  public synchronized void unregisterPriorityProperty(PriorityProperty<?> property) {
-    priorityPropertyMap.remove(property);
-    keyCache = null;
-  }
-
-  public synchronized void unregisterConfigObject(Object configObject) {
-    if (configObject == null) {
-      return;
-    }
-
-    configObjectMap.remove(configObject);
-    keyCache = null;
-  }
-
   public <T> T createConfigObject(Class<T> cls, Object... kvs) {
     Map<String, Object> parameters = new HashMap<>();
     for (int idx = 0; idx < kvs.length; idx += 2) {
@@ -145,7 +131,6 @@ 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/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
index 7598e97..92cd17f 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
@@ -190,7 +190,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     Assert.assertNull(config.booleanValueObj);
     Assert.assertNull(config.getBooleanValueObj1());
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -253,7 +252,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     Assert.assertTrue(config.booleanValueObj);
     Assert.assertTrue(config.getBooleanValueObj1());
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -316,7 +314,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     Assert.assertTrue(config.booleanValueObj);
     Assert.assertTrue(config.getBooleanValueObj1());
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @InjectProperties(prefix = "root")
@@ -363,7 +360,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     Assert.assertEquals(4, config.doubleDef, 0);
     Assert.assertTrue(config.booleanDef);
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -390,7 +386,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     ArchaiusUtils.setProperty("root.low-1.a.high-1.b", Long.MAX_VALUE - 3);
     Assert.assertEquals(Long.MAX_VALUE - 3, config.longValue);
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -406,7 +401,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     ArchaiusUtils.setProperty("root.l1-1", String.valueOf(2f));
     Assert.assertEquals(2f, config.floatValue, 0);
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -418,7 +412,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     ArchaiusUtils.setProperty("root.k.value", "1");
     Assert.assertEquals(1, config.intValue);
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 
   @Test
@@ -437,6 +430,5 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
     ArchaiusUtils.setProperty("override.low", null);
     Assert.assertNull(config.strValue);
 
-    priorityPropertyManager.unregisterConfigObject(config);
   }
 }
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
index fe8604f..43356ed 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
@@ -61,8 +61,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertEquals(-2L, (long) config.getValue());
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -91,8 +89,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertEquals(-2, (int) config.getValue());
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -121,8 +117,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertEquals("def", config.getValue());
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -151,8 +145,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertFalse(config.getValue());
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -181,8 +173,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertEquals(-2, config.getValue(), 0);
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -211,8 +201,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
 
     ArchaiusUtils.setProperty(low, null);
     Assert.assertEquals(-2, config.getValue(), 0);
-
-    priorityPropertyManager.unregisterPriorityProperty(config);
   }
 
   @Test
@@ -226,7 +214,5 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
     config.addConfiguration(new MapConfiguration(Collections.singletonMap(high, "high-value")));
 
     Assert.assertEquals("high-value", property.getValue());
-
-    priorityPropertyManager.unregisterPriorityProperty(property);
   }
 }
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
index 9603e15..841c8a9 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
@@ -18,14 +18,10 @@ package org.apache.servicecomb.config.priority;
 
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
-import org.apache.servicecomb.config.ConfigUtil;
 import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 
-import com.netflix.config.DynamicProperty;
-
 public class TestPriorityPropertyBase {
   protected PriorityPropertyManager priorityPropertyManager;
 
@@ -42,12 +38,6 @@ public class TestPriorityPropertyBase {
 
   @After
   public void teardown() {
-    Assert.assertTrue(priorityPropertyManager.getPriorityPropertyMap().isEmpty());
-    Assert.assertTrue(priorityPropertyManager.getConfigObjectMap().isEmpty());
-    for (DynamicProperty property : ConfigUtil.getAllDynamicProperties().values()) {
-      Assert.assertTrue(ConfigUtil.getCallbacks(property).isEmpty());
-    }
-
     ArchaiusUtils.resetConfig();
   }
 }
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java
new file mode 100644
index 0000000..bccf97f
--- /dev/null
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java
@@ -0,0 +1,109 @@
+/*
+ * 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.config.priority;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.apache.servicecomb.config.ConfigUtil;
+import org.apache.servicecomb.config.inject.InjectProperties;
+import org.apache.servicecomb.config.inject.InjectProperty;
+import org.apache.servicecomb.config.inject.TestConfigObjectFactory;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.netflix.config.DynamicProperty;
+
+public class TestPriorityPropertyManager {
+  String high = "ms.schema.op";
+
+  String middle = "ms.schema";
+
+  String low = "ms";
+
+  String[] keys = {high, middle, low};
+
+  @InjectProperties(prefix = "root")
+  public static class ConfigWithAnnotation {
+    @InjectProperty(prefix = "override", keys = {"high", "low"})
+    public String strValue;
+  }
+
+  @Before
+  public void setup() {
+    // avoid write too many logs
+    Logger.getRootLogger().setLevel(Level.OFF);
+
+    ArchaiusUtils.resetConfig();
+
+    Logger.getRootLogger().setLevel(Level.INFO);
+  }
+
+  @After
+  public void teardown() {
+    ArchaiusUtils.resetConfig();
+  }
+
+  private void waitKeyForGC(PriorityPropertyManager priorityPropertyManager) {
+    long maxTime = 10000;
+    long currentTime = System.currentTimeMillis();
+    while (System.currentTimeMillis() - currentTime < maxTime) {
+      if (priorityPropertyManager.getPriorityPropertySet().isEmpty()
+          && priorityPropertyManager.getConfigObjectMap().isEmpty()) {
+        break;
+      }
+      System.runFinalization();
+      System.gc();
+      try {
+        Thread.yield();
+        Thread.sleep(10);
+      } catch (InterruptedException e) {
+        e.printStackTrace();
+      }
+    }
+  }
+
+  @Test
+  public void testConfigurationsAreGCCollected() {
+    long timeBegin = System.currentTimeMillis();
+
+    PriorityPropertyManager priorityPropertyManager = new PriorityPropertyManager();
+    for (int i = 0; i < 100; i++) {
+      for (int j = 0; j < 100; j++) {
+        TestConfigObjectFactory.ConfigWithAnnotation configConfigObject = priorityPropertyManager.createConfigObject(
+            TestConfigObjectFactory.ConfigWithAnnotation.class);
+        Assert.assertEquals("abc", configConfigObject.strDef);
+        PriorityProperty<Long> configPriorityProperty = priorityPropertyManager.
+            createPriorityProperty(Long.class, -1L, -2L, keys);
+        Assert.assertEquals(-2L, (long) configPriorityProperty.getValue());
+      }
+    }
+
+    waitKeyForGC(priorityPropertyManager);
+
+    Assert.assertTrue(priorityPropertyManager.getPriorityPropertySet().isEmpty());
+    Assert.assertTrue(priorityPropertyManager.getConfigObjectMap().isEmpty());
+    for (DynamicProperty property : ConfigUtil.getAllDynamicProperties().values()) {
+      Assert.assertTrue(ConfigUtil.getCallbacks(property).isEmpty());
+    }
+
+    System.out.println("Token : " + (System.currentTimeMillis() - timeBegin));
+  }
+}
diff --git a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
index f5ec8c5..858c644 100644
--- a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
+++ b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
@@ -24,8 +24,6 @@ import org.slf4j.LoggerFactory;
 public class InspectorBootListener implements BootListener {
   private static final Logger LOGGER = LoggerFactory.getLogger(InspectorBootListener.class);
 
-  private InspectorConfig inspectorConfig;
-
   @Override
   public int getOrder() {
     return Short.MAX_VALUE;
@@ -33,7 +31,8 @@ public class InspectorBootListener implements BootListener {
 
   @Override
   public void onAfterTransport(BootEvent event) {
-    inspectorConfig = event.getScbEngine().getPriorityPropertyManager().createConfigObject(InspectorConfig.class);
+    InspectorConfig inspectorConfig = event.getScbEngine().getPriorityPropertyManager()
+        .createConfigObject(InspectorConfig.class);
     if (!inspectorConfig.isEnabled()) {
       LOGGER.info("inspector is not enabled.");
       return;
@@ -46,12 +45,4 @@ public class InspectorBootListener implements BootListener {
     inspector.setPriorityPropertyManager(event.getScbEngine().getPriorityPropertyManager());
     event.getScbEngine().getProducerProviderManager().registerSchema("inspector", inspector);
   }
-
-  @Override
-  public void onAfterClose(BootEvent event) {
-    // some UT case, inspectorConfig is null
-    if (inspectorConfig != null) {
-      event.getScbEngine().getPriorityPropertyManager().unregisterConfigObject(inspectorConfig);
-    }
-  }
 }
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 6e87be2..a747f2c 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,7 @@ public class InspectorImpl {
     priorityPropertyManager.getConfigObjectMap().values().stream()
         .flatMap(Collection::stream)
         .forEach(p -> views.add(createPriorityPropertyView(p)));
-    priorityPropertyManager.getPriorityPropertyMap().keySet().forEach(p ->
+    priorityPropertyManager.getPriorityPropertySet().forEach(p ->
         views.add(createPriorityPropertyView(p)));
     return views;
   }
diff --git a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
index 9b5df43..5bf1d73 100644
--- a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
+++ b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
@@ -43,7 +43,6 @@ public class TestInspectorBootListener {
 
   @AfterClass
   public static void teardown() {
-    priorityPropertyManager.unregisterConfigObject(inspectorConfig);
     ArchaiusUtils.resetConfig();
   }
 
diff --git a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
index 172905f..a3ea9c4 100644
--- a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
+++ b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
@@ -362,10 +362,6 @@ public class TestInspectorImpl {
         views.get(0).getDynamicProperties().stream().map(DynamicPropertyView::getKey).collect(Collectors.toList()),
         Matchers.contains("high", "low"));
 
-    priorityPropertyManager.unregisterPriorityProperty(priorityProperty);
-    views = inspector.priorityProperties();
-    Assert.assertTrue(views.isEmpty());
-
     priorityPropertyManager.close();
     inspector.setPriorityPropertyManager(null);
   }
@@ -376,7 +372,5 @@ public class TestInspectorImpl {
 
     Map<String, String> schemas = Deencapsulation.getField(inspector, "schemas");
     Assert.assertTrue(schemas.get("schema1").indexOf("/webroot/rest/metrics") > 0);
-
-    inspector.getScbEngine().getPriorityPropertyManager().unregisterConfigObject(inspector.getInspectorConfig());
   }
 }


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

Posted by li...@apache.org.
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;
   }