fix(plugin): isolate match cache across config updates - #7681

Request changes. The core idea — generation-scoped match cache, rewritten-path keys, URI-only caching gate, finally-block cache cleanup — is well designed, but two cascades added to the clean* primitives break the sync layer's per-group refresh protocol. The e2e failure on this PR is that regression, not infrastructure flake; I traced it end to end below.

Blocking 1 — cleanPluginData() now clears selectors and rules (caused the e2e failure)

cleanPluginData() is not only a delete-all primitive. The sync layer uses it as the "clear" half of a per-group refresh:

• admin pushes a PLUGIN-group change (e.g. enabling a plugin) -> gateway PluginDataRefresh.refresh() (shenyu-sync-data-center/shenyu-sync-data-http/src/main/java/org/apache/shenyu/sync/data/http/refresh/PluginDataRefresh.java:70-78) calls refreshPluginDataAll() and then re-caches only the plugins.
• refreshPluginDataAll() -> cleanPluginData() -> with this PR also cleanSelectorData() -> selectorMap.clear() + ruleMap.clear().
• On master, cleanPluginData() cleared pluginMap only, so selectors and rules survived a plugin-group refresh. With the cascade, any plugin enable/disable/update over HTTP sync leaves the gateway with zero selectors and zero rules until the next SELECTOR/RULE push — every request answers -107 Can not find selector / -108 Can not find rule.

Hard evidence from this PR's own failing e2e job (e2e-case-logging-rocketmq, run 38904376053, job 116989492155):

13:26:18  websocket-mode batch: "start loggingRocketMQ plugin" -> tests PASS at 13:27:03
13:27:22  compose switches gateway to http sync mode
13:28:04  gateway: update SELECTOR config (x3), update RULE config (x21)   <- re-registration
13:28:59  gateway: update PLUGIN config (x1)                                <- e2e enables loggingRocketMQ
          (no SELECTOR/RULE update after this point)
13:29:20  GET /http/order/findById?id=123 -> {"code": -107, "message": "divide:Can not find selector, please check your configuration!"}
          -> DividePluginTest.before:80 fails, 2 test cases fail

The websocket batch passed because websocket PluginDataHandler.doUpdate only calls onSubscribe per item — no clear — so only the HTTP-sync path exposes the bug. e2e-case (e2e-http-sync-compose, shenyu-e2e-case-http) would very likely have caught it too, but it was cancelled by the matrix fail-fast after the rocketmq job failed.

Blocking 2 — cleanSelectorData() now clears ruleMap

Same protocol issue one level down. refreshSelectorDataAll() -> cleanSelectorData() runs on any SELECTOR-group change (SelectorDataRefresh.java:70-78), and only selectors are re-cached — so every selector change over HTTP sync drops all rules until the next RULE push. In a full refresh it is worse: DataRefreshFactory.executor iterates its EnumMap in key ordinal order, and ConfigGroupEnum declares PLUGIN, RULE, SELECTOR (shenyu-common/.../ConfigGroupEnum.java:33-48), so the sequence is PLUGIN -> RULE -> SELECTOR: the RULE step refills ruleMap, the SELECTOR step then clears it again and nobody refills it — rules stay empty until an unrelated RULE change arrives.

Suggested fix

Keep the cascades in the per-entity delete paths only — this PR already implements them correctly there:

• removePluginDataByPluginName removes the plugin, its selectors and its rules (correct for a plugin delete).
• removeSelectData(pluginName, selectorId) removes the selector and does ruleMap.remove(selectorId) (correct for a selector delete).
• removeRuleData already handles dangling rules.

What needs to change:

1. cleanPluginData(): drop the cleanSelectorData() call, keep pluginMap.clear() + version++ (master semantics).
2. cleanSelectorData(): drop ruleMap.clear(), keep selectorMap.clear() + version++ (master semantics).

Note the same root cause also affects the websocket full-refresh path: onPluginRefresh -> refreshPluginDataSelf -> cleanPluginDataSelf -> removePluginData per item, which with the cascade also drops each refreshed plugin's selectors and rules. Fixing the two clean* methods addresses that path as well.

What is good (and should be kept)

• The generation-scoped match key (version + rewritten path, rule key scoped to the selected selector) is the right shape for the reported issue, and gating caching on "every enabled candidate uses stable URI conditions" plus skipping matchRestful is conservative in the right direction.
• The per-entity cascades in removePluginDataByPluginName / removeSelectData / removeRuleData (including the dangling-selector/dangling-rule handling) are correct and are where the cascade belongs.
• MatchCacheLifecycleTest covers the interesting lifecycles (priority replacement, dynamic competitors, cross-selector scoping, old-generation refill, handler exceptions). What is missing is coverage of the refresh protocol itself: please add a test that simulates refreshPluginDataAll() followed by plugin-only re-cache and asserts that obtainSelectorData / obtainRuleData still return their data (i.e. the exact scenario the e2e caught). A small test around DataRefreshFactory/PluginDataRefresh ordering would be even better.

CI note

The two red jobs are this PR's regression, not infrastructure: master's e2e is green (runs 38909074537 / 38924255915 both success, including the rocketmq case), and the failure mechanism is reproducible from the code path above. Once the cascades are removed, please rerun the e2e workflow — I would expect the rocketmq case to go green again.

The cache-generation approach is sound. Putting BaseDataCache.version into the match key (version:path for selectors, selectorId.length:selectorId + version:path for rules) means an in-flight request can only populate the generation it started on, so a stale match result can never become visible to a newer configuration — that is the correct invariant, and it is better than trying to invalidate the trie/LRU on every change.

Also correct:

• Caching is now restricted to !matchRestful && onlyUriConditions(...), which is the same safety rule as #8106: only a key that is fully determined by the URI path may be cached. Restful and header/query/time-predicate rules are recomputed every request.
• getRawPath moving to RequestUrlUtils.getRewrittenRawPath keeps the cache key consistent with the rewritten path actually used for matching.
• removePluginDataByPluginName now cascades into selectors and rules, and removeRuleData handles a blank selectorId by sweeping all buckets — both plug real leak paths.

One thing to confirm before merge: version-keyed entries and cache growth. Every configuration change mints a new key prefix, and the old-generation entries become unreachable. If the L1 LRU is bounded that is fine (they age out), but if the L2 trie retains them indefinitely this becomes a slow memory leak proportional to config-change rate. Please confirm the L2 store is bounded or is cleared on version bump — otherwise the fix trades a correctness bug for an unbounded-growth bug.

Minor: removePluginData calls removePluginDataByPluginName (which already increments version) and then increments again; cleanPluginDataSelf no longer cascades into selectors/rules. Both are harmless given version only needs to be monotonic, but the double increment is worth a comment so it does not look accidental.

LGTM once the cache-growth question is answered.

Stream Agent analysis

PR #7681 attempts to isolate match cache across config updates by scoping rule matches to selected selector and BaseDataCache generation. Bounded excerpts and e2e checks show success but full review pending.

Evidence

• PR head SHA 13fb0a1; e2e checks success
• Comment: rebased on 92d16c2; MatchDataCache uses per-plugin maps
• Resource excerpt shenyu-source @13fb0a1b truncated AbstractShenyuPlugin

Recommended next steps

• Review full patch for AbstractShenyuPlugin and MatchDataCache at head 13fb0a1
• Confirm unit tests added for per-selector cache isolation
• Validate with maintainers that finally-block cache clears are correct

Scope: bounded repository evidence; this is not a full code review or a claim that tests were run.

Task: c7b27e72-1cc2-4e7c-8dc7-74ca9b248f42 · Commit: 13fb0a1b5e8d83e9f5713967c7d5ca2d7ec6c54c

Stream Agent analysis

PR #7681 (head 13fb0a1) isolates match cache across config updates with version-scoped keys and selector-scoped negative caching. All partition reviews PASS, complete diff reviewed, CI green (68 checks). No HIGH/CRITICAL defects.

Evidence

• Frozen review: all_parts_passed true, complete_diff true, CI passed 68 checks.
• Two synthesis nodes reduce-1-0 and reduce-1-1 both PASS with no blocking findings.

Recommended next steps

• Merge after standard approvals; monitor cache behavior in staging.

Scope: captured diff and GitHub check metadata only; no local tests were executed.

Task: eed0addd-80d2-4a92-8534-b2208a3c87e6 · Commit: 13fb0a1b5e8d83e9f5713967c7d5ca2d7ec6c54c

Review verdict: PASS

FINAL synthesis: all child partitions PASS, complete_diff true, CI green (68 checks). Cross-file cache isolation changes consistent, thread-safe, with regression tests. No HIGH/CRITICAL defects.

Publication: APPROVE (EXPLICIT_PASS)