Aias00 commented on code in PR #7318:
URL: https://github.com/apache/shenyu/pull/7318#discussion_r4111192069
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/listener/websocket/WebsocketCollector.java:
##########
@@ -307,12 +306,15 @@ private static void removeSessionSendQueue(final Session
session) {
private void clearSession(final Session session) {
SESSION_SET.remove(session);
removeSessionSendQueue(session);
- String namespaceId = getNamespaceId(session);
- if (StringUtils.isNotBlank(namespaceId)) {
- NAMESPACE_SESSION_MAP.getOrDefault(namespaceId,
Sets.newConcurrentHashSet()).remove(session);
- }
+ removeSessionFromNamespaceMap(session);
ThreadLocalUtils.clear();
}
+
+ private static void removeSessionFromNamespaceMap(final Session session) {
+ // clearSession runs from onClose/onError, where the session is
already closed and
+ // getNamespaceId refuses to read it, so the session is swept out of
every namespace set
+ NAMESPACE_SESSION_MAP.values().forEach(sessions ->
sessions.remove(session));
Review Comment:
Correct, and I prefer this to resolving the namespace via `getNamespaceId`:
it is idempotent, needs nothing captured at registration, and still cleans up a
session that reached a namespace set without going through the normal
registration path.
Two notes:
- It is O(number of namespaces) per disconnect. Namespace counts are small,
so this is fine - just worth a word here if that ever stops being true.
- Unlike #7317 this leaves empty sets behind. Bounded by the distinct
namespace count, so not a leak - but that is the difference that lets #7317
prune them, and pruning is only safe because #7317 also moved `onOpen` from
`computeIfAbsent` to `compute` (your `computeIfAbsent` + `add` here is correct
as-is precisely because nothing removes the entry).
My suggestion in the review body: keep this as the fallback branch for a
missing remembered namespace in #7317, so the normal path stays O(1) and the
odd one is still covered.
--
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]
