wu-sheng commented on a change in pull request #7509:
URL: https://github.com/apache/skywalking/pull/7509#discussion_r692956934
##########
File path:
oap-server/server-configuration/configuration-api/src/main/java/org/apache/skywalking/oap/server/configuration/api/ConfigWatcherRegister.java
##########
@@ -82,34 +82,96 @@ void configSync() {
config.getItems().forEach(item -> {
String itemName = item.getName();
WatcherHolder holder = register.get(itemName);
- if (holder != null) {
- 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.
- }
+ 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 {
- 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.
- }
+ // Don't need to notify, stay in null.
}
} else {
- LOGGER.warn("Config {} from configuration center, doesn't
match any watcher, ignore.", itemName);
+ 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.
+ }
}
});
LOGGER.trace("Current configurations after the sync." +
LINE_SEPARATOR + register.toString());
});
+
+ configTable.ifPresent(this::groupConfigsSync);
+ }
+
+ private void groupConfigsSync(ConfigTable config) {
+ config.getGroupItems().forEach(groupConfigItems -> {
+ String groupConfigItemName = groupConfigItems.getName();
+ WatcherHolder holder = register.get(groupConfigItemName);
+
+ if (holder == null || holder.getWatcher().watchType !=
ConfigChangeWatcher.WatchType.GROUP) {
+ LOGGER.warn(
+ "Config {} from configuration center, doesn't match any
WatchType.GROUP watcher, ignore.",
+ groupConfigItemName
+ );
+ return;
+ }
+
+ GroupConfigChangeWatcher watcher = (GroupConfigChangeWatcher)
holder.getWatcher();
+ Map<String, ConfigTable.ConfigItem> groupItems =
groupConfigItems.getItems();
+ groupItems.forEach((groupItemName, groupItem) -> {
+ String newItemValue = groupItem.getValue();
+ if (newItemValue == null) {
+ if (watcher.groupItems().get(groupItemName) != null) {
+ // Notify watcher, the new value is null with delete
event type.
+ watcher.notify(
+ new ConfigChangeWatcher.ConfigChangeEvent(
+ groupItemName,
+ null,
+ ConfigChangeWatcher.EventType.DELETE
+ ));
+ } else {
+ // Don't need to notify, stay in null.
+ }
+ } else { //add and modify
+ if
(!newItemValue.equals(watcher.groupItems().get(groupItemName))) {
+ watcher.notify(new
ConfigChangeWatcher.ConfigChangeEvent(
+ groupItemName,
+ newItemValue,
+ ConfigChangeWatcher.EventType.MODIFY
+ ));
+ } else {
+ // Don't need to notify, stay in the same config value.
+ }
+ }
+ });
+
+ watcher.groupItems().forEach((oldGroupItemName, oldGroupItemValue)
-> {
+ //delete item
+ if (null == groupItems.get(oldGroupItemName)) {
+ // Notify watcher, the item is deleted with delete event
type.
+ watcher.notify(
+ new
ConfigChangeWatcher.ConfigChangeEvent(oldGroupItemName,
+ null,
ConfigChangeWatcher.EventType.DELETE
+ ));
+ }
+ });
+ });
+
+ LOGGER.trace("Current configurations after the sync." + LINE_SEPARATOR
+ register.toString());
Review comment:
There are 2 `Current configurations after the sync.` logs. Even the old
one, it use the `+` without `LOGGER.isTraceEnable`, which has performance
impact.(very little, but exists.
Besides performance, the log should be more clear about more clear, what
exactly is the final status after sync.
--
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-unsubscr...@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
