----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59239/#review175000 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 217 (patched) <https://reviews.apache.org/r/59239/#comment248309> The exception should be logged in `reportDeadLocator`. Although perhaps the message in there should be changed from "locator {0} is not running" to "could not connect to locator {0}". If we keep the log statement, I think it's better to use the other form `logger.info("queryOneLocator got Exception ", ioe);` geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 237 (patched) <https://reviews.apache.org/r/59239/#comment248313> If connecting to the locator fails with an `IOException`, this may be because the locator's IP has changed. Add the locator back to the list of locators using host address rather than IP. This will cause another DNS lookup, hopefully finding the locator. geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 243 (patched) <https://reviews.apache.org/r/59239/#comment248310> This comment should be deleted. geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 248 (patched) <https://reviews.apache.org/r/59239/#comment248311> This could be written as `for (InetSocketAddress tloc : locatorList.getLocators())` and `locIterator` declaration removed. geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 250 (patched) <https://reviews.apache.org/r/59239/#comment248321> In the case where the locator isn't in the list, do we mean to add it to `newLocatorList` or discard it? If we mean to add it, we should move the check `locator.getHostName() != null` to above the loop. geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 257 (patched) <https://reviews.apache.org/r/59239/#comment248312> typo: "loc from" instead of "loc form" ideally comma comes before space in log messages, or I would just say `"from: " + locator + " to: "`. geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java Lines 354 (patched) <https://reviews.apache.org/r/59239/#comment248314> I think this is a good candidate for the "enhanced for statement" sytax (`for (badLoc : badLocators) {`) geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java Line 141 (original), 142 (patched) <https://reviews.apache.org/r/59239/#comment248316> I think you copied the wrong comment here -- we arent' expecting a reply if we take a `replyExpected` parameter. Same with the comment on line 149/150. geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java Lines 155 (patched) <https://reviews.apache.org/r/59239/#comment248318> Is floc2 necessary? - Galen O'Sullivan On May 15, 2017, 6:33 p.m., Hitesh Khamesra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59239/ > ----------------------------------------------------------- > > (Updated May 15, 2017, 6:33 p.m.) > > > Review request for geode, Bruce Schuchardt, Galen O'Sullivan, and Udo > Kohlmeyer. > > > Repository: geode > > > Description > ------- > > GEODE-2804 Update InetSocketAddress, when there is IOException. > > Geode keeps InetSocketAddress for locators. So, if locators ip > changes in cloud/VM enviroment then Geode process unable to > connect to locator. Thus we have fixed this problem in two way. > > a. If Geode client sees IOException while connectin to locator then > we change cached InetAddress to use locator hostname. In this way, > client does DNS query again for locator host. > b. For other Geode process, now we connect to locator using hostname. > > Added couple of junit tests for it. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > 1e807ee > > geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java > 4bf010b > > geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java > 6b54170 > > geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java > 0efba01 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java > d61e72d > > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java > 385569c > > > Diff: https://reviews.apache.org/r/59239/diff/2/ > > > Testing > ------- > > > Thanks, > > Hitesh Khamesra > >
