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


##########
shenyu-client/shenyu-client-core/src/main/java/org/apache/shenyu/client/core/disruptor/subcriber/ShenyuClientURIExecutorSubscriber.java:
##########
@@ -120,4 +127,15 @@ private void sendHeartbeat(final URIRegisterDTO 
uriRegisterDTO) {
         uriRegisterDTO.setInstanceInfo(SystemInfoUtils.getSystemInfo());
         shenyuClientRegisterRepository.sendHeartbeat(uriRegisterDTO);
     }
+
+    private void addUriIfAbsent(final URIRegisterDTO uriRegisterDTO) {
+        boolean alreadyRegistered = uris.stream().anyMatch(registered ->
+                Objects.equals(registered.getNamespaceId(), 
uriRegisterDTO.getNamespaceId())
+                        && Objects.equals(registered.getContextPath(), 
uriRegisterDTO.getContextPath())

Review Comment:
   Question (please answer before merge, not necessarily by changing code): the 
dedup key is (namespaceId, contextPath, host, port), so `appName`, `protocol` 
and `rpcType` are not considered. Is that tuple guaranteed unique inside one 
subscriber instance? If two registrations ever differ only in `appName` (same 
namespace, context path, host and port going through the same client context), 
the second one is silently dropped from the heartbeat list while `persistURI` 
still accepted it, so admin would eventually see it as offline. Including 
`appName` in the predicate costs nothing - happy either way, I just want the 
assumption stated.



-- 
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