----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59546/#review176643 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java Line 277 (original), 281 (patched) <https://reviews.apache.org/r/59546/#comment250029> In this case you leave "hostAddress" null. It is used later (down around line 338) in this code: java.net.InetSocketAddress sockAddr = new java.net.InetSocketAddress(hostAddress, portVal); if (!locs.contains(sockAddr)) { if (!firstUniqueLocator) { sb.append(','); } else { firstUniqueLocator = false; } locs.add(new java.net.InetSocketAddress(hostAddress, portVal)); sb.append(locatorsb); } In this context a null hostAddress many any local address which is not correct. geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java Line 291 (original), 293 (patched) <https://reviews.apache.org/r/59546/#comment250030> What about this UnknownHostException on the bindAddr? I don't remember why we have a bindAddr but do you also need to allow it to be resolved later? geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java Line 262 (original), 263 (patched) <https://reviews.apache.org/r/59546/#comment250020> It is unclear to me how this change helps. We end up using these props to connect the DistributedSystem and end up calling org.apache.geode.internal.AbstractConfig.setAttribute(String, String, ConfigSource) for each one. This method does the following for MCAST_ADDRESS: } else if (valueType.equals(InetAddress.class)) { try { attObjectValue = InetAddress.getByName(attValue); } catch (UnknownHostException ex) { throw new IllegalArgumentException( LocalizedStrings.AbstractConfig_0_VALUE_1_MUST_BE_A_VALID_HOST_NAME_2 .toLocalizedString(new Object[] {attName, attValue, ex.toString()})); } So wouldn't we still get an IllegalArgumentException here? From your description I thought you just wanted to fix the locator list so I'm not sure why the MCAST_ADDRESS change was made. But if it is important it seems like more work is needed down in the AbstractConfig code. - Darrel Schneider On May 25, 2017, 12:06 p.m., Hitesh Khamesra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59546/ > ----------------------------------------------------------- > > (Updated May 25, 2017, 12:06 p.m.) > > > Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo > Kohlmeyer. > > > Repository: geode > > > Description > ------- > > We configure locator list to start the cache. This locator list is validated > while creating the cache. We verify whether locator host exist or not. Now we > have remove this verification as in cloud environment host may not available > for time being. > > Patch from Bruce. Modified couple of tests. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorImpl.java > c1bfc93 > > geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java > 01c6157 > > geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java > 7caad3f > > geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java > 5ab1bed > > geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java > 1dc2fd1 > > geode-core/src/test/java/org/apache/geode/codeAnalysis/AnalyzeSerializablesJUnitTest.java > dc73f04 > > geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java > 9f6c5fb > > geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt > 9cff80d > > geode-wan/src/main/java/org/apache/geode/cache/client/internal/locator/wan/LocatorMembershipListenerImpl.java > d6d5d7c > > > Diff: https://reviews.apache.org/r/59546/diff/2/ > > > Testing > ------- > > > Thanks, > > Hitesh Khamesra > >
