> On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote: > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > > Lines 217 (patched) > > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line219> > > > > 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);`
Good Catch. I have removed that log, otherwise it will fill the log file > On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote: > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > > Lines 237 (patched) > > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line239> > > > > 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. Updated > On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote: > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > > Lines 248 (patched) > > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line250> > > > > This could be written as > > > > `for (InetSocketAddress tloc : locatorList.getLocators())` > > > > and `locIterator` declaration removed. updated > On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote: > > geode-core/src/main/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImpl.java > > Lines 250 (patched) > > <https://reviews.apache.org/r/59239/diff/2/?file=1718520#file1718520line252> > > > > 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. Not sure why I had added that but I put that check up. > On May 15, 2017, 7:43 p.m., Galen O'Sullivan wrote: > > geode-core/src/test/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java > > Lines 155 (patched) > > <https://reviews.apache.org/r/59239/diff/2/?file=1718525#file1718525line155> > > > > Is floc2 necessary? It just to test the logic. - Hitesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59239/#review175000 ----------------------------------------------------------- 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 > >