Re: Review Request 58742: GEODE-2632: minor fixes from code review

2017-04-27 Thread Ken Howe

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58742/#review173224
---


Ship it!




Ship It!

- Ken Howe


On April 26, 2017, 5:17 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58742/
> ---
> 
> (Updated April 26, 2017, 5:17 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Patrick 
> Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Apply fixes for issues from code review. Add TODO comments for larger issues.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  69203117da71e80c753338b048e93de0f6859443 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java 
> 226ffa6cfa3636437011ed41ceadf69b08155a70 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  74ec96c23076b88d009583a5cb778e147b829c09 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
>  e7f5c08cb3451e82dc1d3b23d777665b8fd05884 
> 
> 
> Diff: https://reviews.apache.org/r/58742/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 58742: GEODE-2632: minor fixes from code review

2017-04-26 Thread Kirk Lund


> On April 26, 2017, 5:51 p.m., Jinmei Liao wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
> > Line 2411 (original), 2412 (patched)
> > 
> >
> > I know this is the original logic, but why we would do something like 
> > this:
> > 
> > if (cache==this), return null?

I'm not 100% sure but that's part of the reconnect logic. During reconnect, the 
Geode member closes its Cache and then goes into a mode where it keeps trying 
to reconnect. When it can finally reconnect the Cache comes back up. It's 
possible that the Cache instance still exists during reconnect but is detached 
(ie not available to other classes) in some way (I'm just guessing though). 
Bruce is the best one to ask about this.


- Kirk


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58742/#review173065
---


On April 26, 2017, 5:17 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58742/
> ---
> 
> (Updated April 26, 2017, 5:17 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Patrick 
> Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Apply fixes for issues from code review. Add TODO comments for larger issues.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  69203117da71e80c753338b048e93de0f6859443 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java 
> 226ffa6cfa3636437011ed41ceadf69b08155a70 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  74ec96c23076b88d009583a5cb778e147b829c09 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
>  e7f5c08cb3451e82dc1d3b23d777665b8fd05884 
> 
> 
> Diff: https://reviews.apache.org/r/58742/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 58742: GEODE-2632: minor fixes from code review

2017-04-26 Thread Patrick Rhomberg

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58742/#review173094
---


Ship it!




Ship It!

- Patrick Rhomberg


On April 26, 2017, 5:17 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58742/
> ---
> 
> (Updated April 26, 2017, 5:17 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Patrick 
> Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Apply fixes for issues from code review. Add TODO comments for larger issues.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  69203117da71e80c753338b048e93de0f6859443 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java 
> 226ffa6cfa3636437011ed41ceadf69b08155a70 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  74ec96c23076b88d009583a5cb778e147b829c09 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
>  e7f5c08cb3451e82dc1d3b23d777665b8fd05884 
> 
> 
> Diff: https://reviews.apache.org/r/58742/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 58742: GEODE-2632: minor fixes from code review

2017-04-26 Thread Jinmei Liao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58742/#review173066
---


Ship it!




Ship It!

- Jinmei Liao


On April 26, 2017, 5:17 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58742/
> ---
> 
> (Updated April 26, 2017, 5:17 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, and Patrick 
> Rhomberg.
> 
> 
> Bugs: GEODE-2632
> https://issues.apache.org/jira/browse/GEODE-2632
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Apply fixes for issues from code review. Add TODO comments for larger issues.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
>  69203117da71e80c753338b048e93de0f6859443 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java 
> 226ffa6cfa3636437011ed41ceadf69b08155a70 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  74ec96c23076b88d009583a5cb778e147b829c09 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
>  e7f5c08cb3451e82dc1d3b23d777665b8fd05884 
> 
> 
> Diff: https://reviews.apache.org/r/58742/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>