Sean-Walker0 commented on PR #7364: URL: https://github.com/apache/shenyu/pull/7364#issuecomment-5903519216
Confirmed the downstream behavior — you are right that the null name moves the problem: - `BaseDataCache#removeSelectData` calls `selectorMap.computeIfPresent(data.getPluginName(), ...)`; on the concurrent map a **null key throws NPE**, so the subscriber callback itself can fail rather than merely skip. - `MatchDataCache#removeSelectorData(null, id)` / `#removeRuleDataBySelector(null, id)` hit `SELECTOR_DATA_MAP.get(null)` → null → silent no-op, leaving stale match-cache entries. On the two remedies: **(a) preserve the plugin name** — I could not find a source when the plugin row is already deleted: `SelectorDO` carries only `pluginId`, and `pluginMapper.selectByIds` is the sole lookup feeding `onDeleted`. In the normal cascade (`BatchNamespacePluginDeletedEvent`) the event payload *does* carry the deleted `PluginDO`s, so names exist there; the null case is precisely the dangling-selector path where no row remains anywhere. **(b) selector-id fallback in the data plane** — I sketched it: in `BaseDataCache#removeSelectData`, when `pluginName` is blank, sweep every plugin bucket and drop entries with the matching selector id (buckets are immutable `List.copyOf` results, so via `computeIfPresent` per key); same blank-name sweep in `MatchDataCache#removeSelectorData(pluginName, id)` and `#removeRuleDataBySelector`. However, all three files are currently modified by open PRs — `BaseDataCache` by #7345, `MatchDataCache` by #6336, `CommonPluginDataSubscriber` by #7319/#7088 — so folding the fallback into this admin-scoped PR would create textual conflicts with four in-flight patches. How would you like to proceed? 1. Keep this PR admin-scoped (it fixes the reported 500 and still publishes the delete events), and I send the data-plane fallback as a dedicated follow-up PR built on top of #7345/#6336/#7088 once they land (or immediately, if you prefer it standalone); or 2. Fold the fallback into this PR now despite the overlap. Happy to implement either on your word — the regression test proving a blank-name selector delete purges `BaseDataCache`/`MatchDataCache` is straightforward either way. -- 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]
