Aias00 commented on code in PR #7681:
URL: https://github.com/apache/shenyu/pull/7681#discussion_r4300859527


##########
shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java:
##########
@@ -73,42 +85,50 @@ public static BaseDataCache getInstance() {
      *
      * @param pluginData the plugin data
      */
-    public void cachePluginData(final PluginData pluginData) {
+    public synchronized void cachePluginData(final PluginData pluginData) {
         Optional.ofNullable(pluginData).ifPresent(data -> 
pluginMap.put(data.getName(), data));
+        version++;
     }
     
     /**
      * Remove plugin data.
      *
      * @param pluginData the plugin data
      */
-    public void removePluginData(final PluginData pluginData) {
-        Optional.ofNullable(pluginData).ifPresent(data -> 
pluginMap.remove(data.getName()));
+    public synchronized void removePluginData(final PluginData pluginData) {
+        Optional.ofNullable(pluginData).ifPresent(data -> 
removePluginDataByPluginName(data.getName()));
+        version++;
     }
     
     /**
      * Remove plugin data by plugin name.
      *
      * @param pluginName the plugin name
      */
-    public void removePluginDataByPluginName(final String pluginName) {
+    public synchronized void removePluginDataByPluginName(final String 
pluginName) {
         pluginMap.remove(pluginName);
+        removeSelectDataByPluginName(pluginName);
+        ruleMap.entrySet().removeIf(entry -> 
entry.getValue().stream().anyMatch(rule -> 
pluginName.equals(rule.getPluginName())));
+        version++;
     }
     
     /**
      * Clean plugin data.
      */
-    public void cleanPluginData() {
+    public synchronized void cleanPluginData() {
         pluginMap.clear();
+        cleanSelectorData();

Review Comment:
   [blocking] This cascade breaks the per-group refresh protocol of the sync 
layer and is the direct cause of the e2e failure on this PR. 
`cleanPluginData()` is not only a delete-all primitive: 
`CommonPluginDataSubscriber.refreshPluginDataAll()` 
(CommonPluginDataSubscriber.java:186-188) calls it, and the HTTP sync calls 
that whenever the PLUGIN group changes on admin — `PluginDataRefresh.refresh()` 
(shenyu-sync-data-center/shenyu-sync-data-http/.../refresh/PluginDataRefresh.java:70-78)
 does `refreshPluginDataAll()` and then re-caches only the plugins. On master 
`cleanPluginData()` cleared `pluginMap` only, so selectors/rules survived; with 
this line, every plugin enable/disable/update over HTTP sync empties 
`selectorMap` and `ruleMap`, and only plugins are refilled, so the gateway 
answers -107/-108 to every request until the next SELECTOR/RULE push. 
Reproduced by this PR's own CI: gateway log `13:28:59 update PLUGIN config` 
(the e2e enabling loggingRocketMQ), no SELECTOR/RULE updat
 e afterwards, then `13:29:20 GET /http/order/findById` -> `{"code": -107, 
"message": "divide:Can not find selector"}`. The websocket-mode batch of the 
same e2e passed because websocket `PluginDataHandler.doUpdate` only calls 
`onSubscribe` without any clear. Please drop this cascade (keep `version++`) 
and leave cross-map cleanup to the per-entity delete paths, which this PR 
already implements correctly in `removePluginDataByPluginName`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to