[#5013] Fixing the issue where microservices cannot be registered when enabling RBAC authentication in a dual-engine disaster recovery scenario. #5019
Conversation
…ed when enabling RBAC authentication in a dual-engine disaster recovery scenario.
| addressAutoRefreshed = addresses.stream().anyMatch(addr -> addr.contains(ZONE) || addr.contains(REGION)); | ||
| for (String address : addresses) { | ||
| // Compatible IpPortManager init address is 127.0.0.1:30100 | ||
| if (!address.startsWith("http")) { |
There was a problem hiding this comment.
Used only to determine whether an protocol is included, not to determine the type of protocol.
| tempList.add(endpoint.toString()); | ||
| buildAffinityAddress(endpoint, ownRegion, ownAvailableZone); | ||
| } | ||
| this.addresses.addAll(isFormat ? this.transformAddress(tempList) : tempList); |
There was a problem hiding this comment.
addresses items will duplicate？ or else change to Set , if order is not needed
There was a problem hiding this comment.
need order, and duplicate addresses are acceptable.
| private void buildAffinityAddress(URLEndPoint endpoint, String ownRegion, String ownAvailableZone) { | ||
| if (addressAutoRefreshed) { | ||
| if (regionAndAZMatch(ownRegion, ownAvailableZone, endpoint.getFirst(REGION), endpoint.getFirst(ZONE))) { | ||
| availableZone.add(endpoint.toString()); |
There was a problem hiding this comment.
toString is not Recommendation， the result will changed when the class field added.
There was a problem hiding this comment.
The toString method in URLEndPoint has been overridden, and properties can be added when needed.
There was a problem hiding this comment.
the availableZone mean always not changed when URLEndPoint add properties?
There was a problem hiding this comment.
URLEndPoint is just a utility class, the SC address only requires the protocol+ip+port
| signRequest.setEndpoint(uri); | ||
| return signRequest; | ||
| } catch (Exception e) { | ||
| return null; |
| if (StringUtils.isEmpty(address)) { | ||
| address = registryName; | ||
| } | ||
| synchronized (LOCK) { |
There was a problem hiding this comment.
use caffeine instead ? sync lock is Not recommended. it just a read operation
There was a problem hiding this comment.
The cache loads tokens when they are not stored during queries, and it involves asynchronous tasks, which may lead to concurrency issues. Caffeine without having trie, it is unknown whether there are any bugs.
| } | ||
|
|
||
| private boolean isEngineEndpointsChanged(Set<String> lastEngineEndpoints, Set<String> currentEngineEndpoints) { | ||
| if (lastEngineEndpoints == null || lastEngineEndpoints.isEmpty()) { |
There was a problem hiding this comment.
currentEngineEndpoints will null?
| if (StringUtils.isEmpty(address)) { | ||
| address = registryName; | ||
| } | ||
| synchronized (LOCK) { |
There was a problem hiding this comment.
synchronized is not recommended now. It's not compatible with JDK21 virtual thread feature. Better to use 'ReentrantLock` or other mechanism. see https://openjdk.org/jeps/444
…ed when enabling RBAC authentication in a dual-engine disaster recovery scenario. (apache#5019)
…ed when enabling RBAC authentication in a dual-engine disaster recovery scenario. (apache#5019)
3 participants
No description provided.