optimize: optimize NacosConfiguration singleton reload (#6763)

apache · Aug 18, 2024 · ff1fa8f · ff1fa8f
1 parent b7fc0c1
commit ff1fa8f
Show file tree

Hide file tree

Showing 13 changed files with 405 additions and 50 deletions.
diff --git a/changes/en-us/2.x.md b/changes/en-us/2.x.md
@@ -57,6 +57,7 @@ Add changes here for all PR submitted to the 2.x branch.
 - [[#6748](https://github.com/apache/incubator-seata/pull/6748)] optimize ConsistentHashLoadBalance Algorithm
 - [[#6747](https://github.com/apache/incubator-seata/pull/6747)] optimize fastjson deserialization
 - [[#6755](https://github.com/apache/incubator-seata/pull/6755)] optimize namingserver code logic
+- [[#6763](https://github.com/apache/incubator-seata/pull/6763)] optimize NacosConfiguration singleton reload
 - [[#6761](https://github.com/apache/incubator-seata/pull/6761)] optimize the namingserver code to improve readability
 
 

diff --git a/changes/zh-cn/2.x.md b/changes/zh-cn/2.x.md
@@ -58,8 +58,10 @@
 - [[#6748](https://github.com/apache/incubator-seata/pull/6748)] 优化 ConsistentHashLoadBalance 算法
 - [[#6747](https://github.com/apache/incubator-seata/pull/6747)] 优化 fastjson 反序列化
 - [[#6755](https://github.com/apache/incubator-seata/pull/6755)] 优化namingserver代码逻辑
+- [[#6763](https://github.com/apache/incubator-seata/pull/6763)] 优化 NacosConfiguration 单例加载
 - [[#6761](https://github.com/apache/incubator-seata/pull/6761)] 提升namingserver manager代码可读性
 
+
 ### refactor:
 
 

diff --git a/config/seata-config-core/src/main/java/org/apache/seata/config/Dispose.java b/config/seata-config-core/src/main/java/org/apache/seata/config/Dispose.java
@@ -0,0 +1,27 @@
+/*
+ * 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.seata.config;
+
+/**
+ * The interface Dispose.
+ */
+public interface Dispose {
+    /**
+     * Dispose.
+     */
+    void dispose();
+}
diff --git a/...ig/seata-config-nacos/src/main/java/org/apache/seata/config/nacos/NacosConfiguration.java b/...ig/seata-config-nacos/src/main/java/org/apache/seata/config/nacos/NacosConfiguration.java
@@ -38,6 +38,7 @@
 import org.apache.seata.config.ConfigurationChangeListener;
 import org.apache.seata.config.ConfigurationFactory;
 import org.apache.seata.config.ConfigurationKeys;
+import org.apache.seata.config.Dispose;
 import org.apache.seata.config.processor.ConfigProcessor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -47,7 +48,7 @@
  * The type Nacos configuration.
  *
  */
-public class NacosConfiguration extends AbstractConfiguration {
+public class NacosConfiguration extends AbstractConfiguration implements Dispose {
     private static volatile NacosConfiguration instance;
 
     private static final Logger LOGGER = LoggerFactory.getLogger(NacosConfiguration.class);
@@ -66,10 +67,10 @@ public class NacosConfiguration extends AbstractConfiguration {
     private static final String RAM_ROLE_NAME_KEY = "ramRoleName";
     private static final String USE_PARSE_RULE = "false";
     private static final String CONTEXT_PATH = "contextPath";
-    private static final Configuration FILE_CONFIG = ConfigurationFactory.CURRENT_FILE_INSTANCE;
+    private Configuration fileConfig = ConfigurationFactory.CURRENT_FILE_INSTANCE;
     private static volatile ConfigService configService;
     private static final int MAP_INITIAL_CAPACITY = 8;
-    private static final ConcurrentMap<String, ConcurrentMap<ConfigurationChangeListener, NacosListener>> CONFIG_LISTENERS_MAP
+    private static volatile ConcurrentMap<String, ConcurrentMap<ConfigurationChangeListener, NacosListener>> CONFIG_LISTENERS_MAP
             = new ConcurrentHashMap<>(MAP_INITIAL_CAPACITY);
     private static volatile Properties seataConfig = new Properties();
 
@@ -203,14 +204,14 @@ public Set<ConfigurationChangeListener> getConfigListeners(String dataId) {
         }
     }
 
-    private static Properties getConfigProperties() {
+    private Properties getConfigProperties() {
         Properties properties = new Properties();
         properties.setProperty(ConfigurationKeys.IS_USE_CLOUD_NAMESPACE_PARSING, USE_PARSE_RULE);
         properties.setProperty(ConfigurationKeys.IS_USE_ENDPOINT_PARSING_RULE, USE_PARSE_RULE);
         if (System.getProperty(PRO_SERVER_ADDR_KEY) != null) {
             properties.setProperty(PRO_SERVER_ADDR_KEY, System.getProperty(PRO_SERVER_ADDR_KEY));
         } else {
-            String address = FILE_CONFIG.getConfig(getNacosAddrFileKey());
+            String address = fileConfig.getConfig(getNacosAddrFileKey());
             if (address != null) {
                 properties.setProperty(PRO_SERVER_ADDR_KEY, address);
             }
@@ -219,7 +220,7 @@ private static Properties getConfigProperties() {
         if (System.getProperty(PRO_NAMESPACE_KEY) != null) {
             properties.setProperty(PRO_NAMESPACE_KEY, System.getProperty(PRO_NAMESPACE_KEY));
         } else {
-            String namespace = FILE_CONFIG.getConfig(getNacosNameSpaceFileKey());
+            String namespace = fileConfig.getConfig(getNacosNameSpaceFileKey());
             if (namespace == null) {
                 namespace = DEFAULT_NAMESPACE;
             }
@@ -228,7 +229,7 @@ private static Properties getConfigProperties() {
         if (!initNacosAuthProperties(properties)) {
             LOGGER.info("Nacos config auth properties empty.");
         }
-        String contextPath = StringUtils.isNotBlank(System.getProperty(CONTEXT_PATH)) ? System.getProperty(CONTEXT_PATH) : FILE_CONFIG.getConfig(getNacosContextPathKey());
+        String contextPath = StringUtils.isNotBlank(System.getProperty(CONTEXT_PATH)) ? System.getProperty(CONTEXT_PATH) : fileConfig.getConfig(getNacosContextPathKey());
         if (StringUtils.isNotBlank(contextPath)) {
             properties.setProperty(CONTEXT_PATH, contextPath);
         }
@@ -242,21 +243,21 @@ private static Properties getConfigProperties() {
      * @param sourceProperties the source properties
      * @return auth properties
      */
-    private static boolean initNacosAuthProperties(Properties sourceProperties) {
-        String userName = StringUtils.isNotBlank(System.getProperty(USER_NAME)) ? System.getProperty(USER_NAME) : FILE_CONFIG.getConfig(getNacosUserName());
+    private boolean initNacosAuthProperties(Properties sourceProperties) {
+        String userName = StringUtils.isNotBlank(System.getProperty(USER_NAME)) ? System.getProperty(USER_NAME) : fileConfig.getConfig(getNacosUserName());
         if (StringUtils.isNotBlank(userName)) {
-            String password = StringUtils.isNotBlank(System.getProperty(PASSWORD)) ? System.getProperty(PASSWORD) : FILE_CONFIG.getConfig(getNacosPassword());
+            String password = StringUtils.isNotBlank(System.getProperty(PASSWORD)) ? System.getProperty(PASSWORD) : fileConfig.getConfig(getNacosPassword());
             if (StringUtils.isNotBlank(password)) {
                 sourceProperties.setProperty(USER_NAME, userName);
                 sourceProperties.setProperty(PASSWORD, password);
                 LOGGER.info("Nacos check auth with userName/password.");
                 return true;
             }
         } else {
-            String accessKey = StringUtils.isNotBlank(System.getProperty(ACCESS_KEY)) ? System.getProperty(ACCESS_KEY) : FILE_CONFIG.getConfig(getNacosAccessKey());
-            String ramRoleName = StringUtils.isNotBlank(System.getProperty(RAM_ROLE_NAME_KEY)) ? System.getProperty(RAM_ROLE_NAME_KEY) : FILE_CONFIG.getConfig(getNacosRamRoleNameKey());
+            String accessKey = StringUtils.isNotBlank(System.getProperty(ACCESS_KEY)) ? System.getProperty(ACCESS_KEY) : fileConfig.getConfig(getNacosAccessKey());
+            String ramRoleName = StringUtils.isNotBlank(System.getProperty(RAM_ROLE_NAME_KEY)) ? System.getProperty(RAM_ROLE_NAME_KEY) : fileConfig.getConfig(getNacosRamRoleNameKey());
             if (StringUtils.isNotBlank(accessKey)) {
-                String secretKey = StringUtils.isNotBlank(System.getProperty(SECRET_KEY)) ? System.getProperty(SECRET_KEY) : FILE_CONFIG.getConfig(getNacosSecretKey());
+                String secretKey = StringUtils.isNotBlank(System.getProperty(SECRET_KEY)) ? System.getProperty(SECRET_KEY) : fileConfig.getConfig(getNacosSecretKey());
                 if (StringUtils.isNotBlank(secretKey)) {
                     sourceProperties.put(ACCESS_KEY, accessKey);
                     sourceProperties.put(SECRET_KEY, secretKey);
@@ -310,15 +311,15 @@ public static String getNacosRamRoleNameKey() {
         return String.join(ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR, ConfigurationKeys.FILE_ROOT_CONFIG, CONFIG_TYPE, RAM_ROLE_NAME_KEY);
     }
 
-    private static String getNacosGroup() {
-        return FILE_CONFIG.getConfig(getNacosGroupKey(), DEFAULT_GROUP);
+    private String getNacosGroup() {
+        return fileConfig.getConfig(getNacosGroupKey(), DEFAULT_GROUP);
     }
 
-    private static String getNacosDataId() {
-        return FILE_CONFIG.getConfig(getNacosDataIdKey(), DEFAULT_DATA_ID);
+    private String getNacosDataId() {
+        return fileConfig.getConfig(getNacosDataIdKey(), DEFAULT_DATA_ID);
     }
 
-    private static String getNacosDataType() {
+    private String getNacosDataType() {
         return ConfigProcessor.resolverConfigDataType(getNacosDataId());
     }
 
@@ -339,7 +340,7 @@ private static String getNacosContextPathKey() {
         return String.join(ConfigurationKeys.FILE_CONFIG_SPLIT_CHAR, ConfigurationKeys.FILE_ROOT_CONFIG, CONFIG_TYPE, CONTEXT_PATH);
     }
 
-    private static void initSeataConfig() {
+    private void initSeataConfig() {
         try {
             String nacosDataId = getNacosDataId();
             String config = configService.getConfig(nacosDataId, getNacosGroup(), DEFAULT_CONFIG_TIMEOUT);
@@ -360,10 +361,27 @@ public String getTypeName() {
         return CONFIG_TYPE;
     }
 
+    @Override
+    public void dispose() {
+        if (null != CONFIG_LISTENERS_MAP) {
+            CONFIG_LISTENERS_MAP.clear();
+        }
+        if (null != seataConfig) {
+            seataConfig.clear();
+        }
+        if (null != configService) {
+            configService = null;
+        }
+        if (null != instance) {
+            instance = null;
+        }
+        fileConfig = ConfigurationFactory.CURRENT_FILE_INSTANCE;
+    }
+
     /**
      * Non-blocking subscriptions prohibit adding subscriptions in the thread pool to prevent thread termination
      */
-    public static class NacosListener extends AbstractSharedListener {
+    public class NacosListener extends AbstractSharedListener {
         private final String dataId;
         private final ConfigurationChangeListener listener;
 

diff --git a/...eata/config/nacos/NacosConfiguration.java → ...ata/config/extend/NacosConfiguration.java b/...eata/config/nacos/NacosConfiguration.java → ...ata/config/extend/NacosConfiguration.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.seata.config.nacos;
+package io.seata.config.extend;
 
 import java.io.IOException;
 import java.util.Enumeration;
@@ -23,8 +23,6 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 
 import com.alibaba.nacos.api.NacosFactory;
 import com.alibaba.nacos.api.config.ConfigService;
@@ -245,7 +243,6 @@ public static String getNacosDataIdKey() {
             NACOS_DATA_ID_KEY);
     }
 
-
     private static String getNacosGroup() {
         return FILE_CONFIG.getString(getNacosGroupKey());
     }
@@ -316,17 +313,17 @@ public void innerReceive(String dataId, String group, String configInfo) {
                         }
                     }
                     //Get all the monitored dataids and judge whether it has been modified
-                    for (Map.Entry<String, ConcurrentMap<ConfigurationChangeListener, NacosListener>> entry : CONFIG_LISTENERS_MAP.entrySet()) {
+                    for (Map.Entry<String, ConcurrentMap<ConfigurationChangeListener, NacosListener>> entry :
+                        CONFIG_LISTENERS_MAP.entrySet()) {
                         String listenedDataId = entry.getKey();
                         String propertyOld = seataConfig.getProperty(listenedDataId, "");
                         String propertyNew = seataConfigNew.getProperty(listenedDataId, "");
                         if (!propertyOld.equals(propertyNew)) {
-                            ConfigurationChangeEvent event =
-                                new ConfigurationChangeEvent().setDataId(listenedDataId).setNewValue(propertyNew)
-                                    .setNamespace(group);
+                            ConfigurationChangeEvent event = new ConfigurationChangeEvent().setDataId(listenedDataId)
+                                .setNewValue(propertyNew).setNamespace(group);
 
-                            ConcurrentMap<ConfigurationChangeListener, NacosListener> configListeners =
-                                entry.getValue();
+                            ConcurrentMap<ConfigurationChangeListener, NacosListener> configListeners
+                                = entry.getValue();
                             for (ConfigurationChangeListener configListener : configListeners.keySet()) {
                                 configListener.onProcessEvent(event);
                             }
@@ -340,8 +337,8 @@ public void innerReceive(String dataId, String group, String configInfo) {
                 LOGGER.error("innerReceive error: {}", e.getMessage(), e);
             }
             //Compatible with old writing
-            ConfigurationChangeEvent event =
-                new ConfigurationChangeEvent().setDataId(dataId).setNewValue(configInfo).setNamespace(group);
+            ConfigurationChangeEvent event = new ConfigurationChangeEvent().setDataId(dataId).setNewValue(configInfo)
+                .setNamespace(group);
             listener.onProcessEvent(event);
         }
     }

diff --git a/...fig/nacos/NacosConfigurationProvider.java → ...ig/extend/NacosConfigurationProvider.java b/...fig/nacos/NacosConfigurationProvider.java → ...ig/extend/NacosConfigurationProvider.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.seata.config.nacos;
+package io.seata.config.extend;
 
 import io.seata.config.Configuration;
 import org.apache.seata.common.loader.LoadLevel;

diff --git a/...ata/config/nacos/TestConfigCustomSPI.java → ...onfig/extend/TestConfigFromExtendSPI.java b/...ata/config/nacos/TestConfigCustomSPI.java → ...onfig/extend/TestConfigFromExtendSPI.java
@@ -14,19 +14,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.seata.config.nacos;
+package io.seata.config.extend;
 
 import java.security.SecureRandom;
-import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
 import com.alibaba.nacos.api.NacosFactory;
 import com.alibaba.nacos.api.config.ConfigService;
-import com.alibaba.nacos.api.config.listener.AbstractSharedListener;
 import com.alibaba.nacos.api.exception.NacosException;
 
 import com.typesafe.config.Config;
@@ -42,7 +38,7 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
-public class TestConfigCustomSPI {
+public class TestConfigFromExtendSPI {
 
     private static  Config FILE_CONFIG;
     private static ConfigService configService;
@@ -67,7 +63,7 @@ public void testGetConfigProperties() throws Exception {
         Assertions.assertNotNull(configService);
         Configuration configuration = ConfigurationFactory.getInstance();
         String postfix = generateRandomString();
-        String dataId = "nacos.config.custom.spi." + postfix;
+        String dataId = "nacos.config.extension.spi." + postfix;
         String group = FILE_CONFIG.getString("config.test.group");
         String content = "seata";
         CountDownLatch listenerCountDown = new CountDownLatch(1);

diff --git a/...eata-config-nacos/src/test/java/org/apache/seata/config/nacos/NacosConfigurationTest.java b/...eata-config-nacos/src/test/java/org/apache/seata/config/nacos/NacosConfigurationTest.java
@@ -19,26 +19,45 @@
 import java.lang.reflect.Method;
 import java.util.Properties;
 
+import com.alibaba.nacos.api.exception.NacosException;
+
 import org.apache.seata.common.util.ReflectionUtil;
-import org.assertj.core.api.Assertions;
+import org.apache.seata.config.Configuration;
+import org.apache.seata.config.ConfigurationFactory;
+import org.apache.seata.config.Dispose;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
-
 /**
  * The type Nacos configuration test
- *
  */
 public class NacosConfigurationTest {
 
+    private static Configuration configuration;
+
+    @BeforeAll
+    public static void setup() throws NacosException {
+        System.clearProperty("seataEnv");
+        configuration = NacosConfiguration.getInstance();
+        if (configuration instanceof Dispose) {
+            ((Dispose)configuration).dispose();
+        }
+        ConfigurationFactory.reload();
+        configuration = NacosConfiguration.getInstance();
+    }
+
     @Test
     public void testGetConfigProperties() throws Exception {
+        Assertions.assertNotNull(configuration);
         Method method = ReflectionUtil.getMethod(NacosConfiguration.class, "getConfigProperties");
-        Properties properties = (Properties) ReflectionUtil.invokeMethod(null, method);
-        Assertions.assertThat(properties.getProperty("contextPath")).isEqualTo("/bar");
+        //do not use `ConfigurationFactory.getInstance()`, it's a proxy object
+        Properties properties = (Properties)method.invoke(configuration);
+        Assertions.assertEquals("/bar", properties.getProperty("contextPath"));
         System.setProperty("contextPath", "/foo");
-        properties = (Properties) ReflectionUtil.invokeMethod(null, method);
-        Assertions.assertThat(properties.getProperty("contextPath")).isEqualTo("/foo");
+        properties = (Properties)method.invoke(configuration);
+        Assertions.assertEquals("/foo", properties.getProperty("contextPath"));
+        System.clearProperty("contextPath");
     }
 
-
 }