Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

【配置变更】ConfigChangeAspect#configChangeServiceHandle() 方法中,调用configChangeRequestsetArg()时,key设置存在问题 #11968

Closed
Happy-26 opened this issue Apr 16, 2024 · 4 comments · Fixed by #11971
Labels
kind/bug Category issues or prs related to bug.

Comments

@Happy-26
Copy link
Contributor

在前处理事件,设置configChangeRequest.setArg("pluginProperties", properties);,在后处理事件中,也设置configChangeRequest.setArg(ConfigChangeConstants.PLUGIN_PROPERTIES, properties);
同时多个插件的自定义属性均使用同一个key来存储,这导致如果一个操作同时需要被多个插件处理,后边的插件将无法获取到自定义的属性。

  // before plugin service execute
  for (ConfigChangePluginService ccs : beforeExecutePluginServices) {
      final String serviceType = ccs.getServiceType().toLowerCase(Locale.ROOT);
      final Properties properties = configChangeConfigs.getPluginProperties(serviceType);
      configChangeRequest.setArg("pluginProperties", properties);
      ccs.execute(configChangeRequest, configChangeResponse);
      if (null != configChangeResponse.getArgs()) {
          // update args by filter with whitelist
          args = configChangeResponse.getArgs();
      }
      // prevent execute next before plugins service
      if (!configChangeResponse.isSuccess()) {
          retVal = wrapErrorResp(configChangeResponse);
          break;
      }
  }


  // after plugin service execute
  ConfigExecutor.executeAsyncConfigChangePluginTask(() -> {
      for (ConfigChangePluginService ccs : afterExecutePluginServices) {
          try {
              final String serviceType = ccs.getServiceType().toLowerCase(Locale.ROOT);
              final Properties properties = configChangeConfigs.getPluginProperties(serviceType);
              configChangeRequest.setArg(ConfigChangeConstants.PLUGIN_PROPERTIES, properties);
              ccs.execute(configChangeRequest, configChangeResponse);
          } catch (Throwable throwable) {
              LOGGER.warn("execute async plugin services failed {}", throwable.getMessage());
          }
      }
  });

感觉可以修改为,将serviceTpye作为key,存储每个插件自己的自定义属性。
configChangeRequest.setArg(serviceType, properties);

@KomachiSion
Copy link
Collaborator

并不需要, 插件的直线逻辑是每个执行插件在执行前,获取一次对应的配置,然后放入request中, 在下一个插件执行前会重新后去并覆盖,

这也就意味着只需要一个key即可, 每个插件执行时,这个key都是他自身的properties。

@Happy-26
Copy link
Contributor Author

并不需要, 插件的直线逻辑是每个执行插件在执行前,获取一次对应的配置,然后放入request中, 在下一个插件执行前会重新后去并覆盖,

这也就意味着只需要一个key即可, 每个插件执行时,这个key都是他自身的properties。

这就是另一个问题了,现在在setArgs时,使用的putIfAbsent,应为使用的都是一个key,所有后边的插件的properties并不会被设置进去。

@Happy-26
Copy link
Contributor Author

setArg

如果使用一个key,则需要将putIfAbsent替换为put

@KomachiSion
Copy link
Collaborator

那可能修复方式改成用put比较好, 从语意上 set应该是有覆盖含义的

@KomachiSion KomachiSion added the kind/bug Category issues or prs related to bug. label Apr 17, 2024
KomachiSion pushed a commit that referenced this issue Apr 22, 2024
…() in ConfigChangeRequest (#11971)

* Fixed an issue where the maximum number of anti-fragile plug-ins implemented by default in Nacos is one more than the actual number of connections.

* bug fix : 修改删除空服务实例时,服务名和分组名没有正确解析的问题

* Update ConfigChangeAspect.java

修改configChangeRequest.setArg()的key为serviceType

* Update ConfigChangeAspect.java

* Update ConfigChangeRequest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Category issues or prs related to bug.
Projects
None yet
2 participants