bschuchardt commented on a change in pull request #6013:
URL: https://github.com/apache/geode/pull/6013#discussion_r577063677
##########
File path:
geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorHelper.java
##########
@@ -46,13 +47,42 @@ public static boolean addLocator(int distributedSystemId,
DistributionLocatorId
Set<DistributionLocatorId> existingValue =
allLocatorsInfo.putIfAbsent(distributedSystemId, locatorsSet);
if (existingValue != null) {
- if (!existingValue.contains(locator)) {
+ if (!locator.getMemberName().equals(DistributionConfig.DEFAULT_NAME)) {
+ DistributionLocatorId existingLocator =
+ getLocatorWithSameMemberName(existingValue, locator);
+
+ if (existingLocator != null) {
+ // if locator with same name exist, check did all parameters are same
+ if (!locator.detailCompare(existingLocator)) {
+ // some parameters had changed for existing locator
+ // replace it
+ existingValue.remove(existingLocator);
+ ConcurrentHashMap<Integer, Set<String>> allServerLocatorsInfo =
+ (ConcurrentHashMap<Integer, Set<String>>) locatorListener
+ .getAllServerLocatorsInfo();
+ Set<String> alllocators =
allServerLocatorsInfo.get(distributedSystemId);
+ alllocators.remove(existingLocator.toString());
+ existingValue.add(locator);
+ addServerLocator(distributedSystemId, locatorListener, locator);
+ locatorListener.locatorJoined(distributedSystemId, locator,
sourceLocator);
+ return true;
+ }
+ return false;
+ }
existingValue.add(locator);
Review comment:
This code looks wrong to me. The old code would not invoke addLocator
if "existingValue.contains(locator)" returned false. You still need this check
if the check "if (existingLocator != null)" returns false.
if (existingLocator != null) {
your new code
} else if (!existingValue.contains(locator)) {
existingValue.add(locator);
addServerLocator(...);
locatorListener.locatorJoined(...);
}
The way you've got it coded a new locator with the same host:port but a
different memberName will be added as a new locator even if existingValue
contains its ID (because equals ignores the member name). This will cause new
threads to be spawned to communicate with that locator, whereas the old code
wouldn't do that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
