[GitHub] [skywalking] wu-sheng commented on a change in pull request #7509: Support collection type in dynamic configuration core and add zookeeper implementation

[GitBox <gi...@apache.org>]

wu-sheng commented on a change in pull request #7509:
URL: https://github.com/apache/skywalking/pull/7509#discussion_r693709870



##########
File path: oap-server/server-configuration/configuration-api/src/main/java/org/apache/skywalking/oap/server/configuration/api/ConfigWatcherRegister.java
##########
@@ -75,45 +88,127 @@ public void start() {
     }
 
     void configSync() {
-        Optional<ConfigTable> configTable = readConfig(register.keys());
+        singleConfigsSync();
+        groupConfigsSync();
+    }
+
+    private void singleConfigsSync() {
+        Optional<ConfigTable> configTable = readConfig(singleConfigChangeWatcherRegister.keys());
 
         // Config table would be null if no change detected from the implementation.
         configTable.ifPresent(config -> {
             config.getItems().forEach(item -> {
                 String itemName = item.getName();
-                WatcherHolder holder = register.get(itemName);
-                if (holder != null) {
-                    ConfigChangeWatcher watcher = holder.getWatcher();
-                    String newItemValue = item.getValue();
+                WatcherHolder holder = singleConfigChangeWatcherRegister.get(itemName);
+                if (holder == null) {
+                    LOGGER.warn(
+                        "Config {} from configuration center, doesn't match any WatchType.SINGLE watcher, ignore.",
+                        itemName
+                    );
+                    return;
+                }
+                ConfigChangeWatcher watcher = holder.getWatcher();
+                String newItemValue = item.getValue();
+                if (newItemValue == null) {
+                    if (watcher.value() != null) {
+                        // Notify watcher, the new value is null with delete event type.
+                        watcher.notify(
+                            new ConfigChangeWatcher.ConfigChangeEvent(null, ConfigChangeWatcher.EventType.DELETE));
+                    } else {
+                        // Don't need to notify, stay in null.
+                    }
+                } else {
+                    if (!newItemValue.equals(watcher.value())) {
+                        watcher.notify(new ConfigChangeWatcher.ConfigChangeEvent(
+                            newItemValue,
+                            ConfigChangeWatcher.EventType.MODIFY
+                        ));
+                    } else {
+                        // Don't need to notify, stay in the same config value.
+                    }
+                }
+            });
+            if (LOGGER.isTraceEnabled()) {

Review comment:
       `LOGGER` is not recommended, please change to `@Slf4j`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org