Aias00 commented on code in PR #7267:
URL: https://github.com/apache/shenyu/pull/7267#discussion_r4110185162
##########
shenyu-admin/src/main/resources/mappers/discovery-handler-sqlmap.xml:
##########
@@ -69,6 +69,17 @@
from discovery_handler
</select>
+ <select id="selectAllByNamespaceId" resultMap="BaseResultMap">
+ SELECT <include refid="Base_Column_List"/>
+ FROM discovery_handler
+ WHERE id IN (
+ SELECT discovery_handler_id FROM discovery_rel
+ WHERE selector_id IN (SELECT id FROM selector WHERE namespace_id =
#{namespaceId})
+ OR ((selector_id IS NULL OR selector_id = '')
+ AND proxy_selector_id IN (SELECT id FROM proxy_selector
WHERE namespace_id = #{namespaceId}))
Review Comment:
Question (non-blocking): the two branches here are ORed, but `buildSyncData`
resolves a handler through whichever of the two ids has length and prefers
`selector_id`. With this predicate a relation row that carries **both**
`selector_id` and `proxy_selector_id` is published to the namespace of its
selector *and* to the namespace of its proxy selector, even though only one of
them is what the service will actually use at runtime.
Is a row with both ids reachable from admin today? If yes, please resolve
the effective namespace with a CASE/COALESCE mirroring the runtime precedence,
so the database view cannot disagree with what Netty-side sync will bind. If
not, saying so here (or in the javadoc above the mapper method) is enough.
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/AbstractDataChangedListener.java:
##########
@@ -351,14 +351,14 @@ protected void refreshLocalCache() {
* Update selector cache.
*/
protected void updateSelectorCache(final String namespaceId) {
- this.updateCache(ConfigGroupEnum.SELECTOR, selectorService.listAll(),
namespaceId);
+ this.updateCache(ConfigGroupEnum.SELECTOR,
selectorService.listAllByNamespaceId(namespaceId), namespaceId);
Review Comment:
Non-blocking observation for whoever reviews the release note: this is the
line that actually changes gateway behaviour. Before this patch every namespace
cache was built from `selectorService.listAll()` and then keyed by
`namespaceId`, so a bootstrap that asked for namespace A received selectors
belonging to A **and** every other namespace; from here on it receives A only.
That is the #6585 fix, and it means:
- a handler whose bound selector or proxy selector row has been deleted now
disappears from every namespace instead of NPEing `listAll()` (WARN-level log
would help here),
- anything still keyed by **(namespaceId, group)** downstream must be
complete per namespace, otherwise those rows are silently invisible.
I checked all seven `listAllByNamespaceId` methods this switch needs already
exist on master, and `refreshLocalCache`
(AbstractDataChangedListener.java:335-348) already loops namespaces, so there
is no remaining full-sync caller left behind.
--
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]
