> 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
> 
>

Reply via email to