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




geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
 (line 1154)
<https://reviews.apache.org/r/53745/#comment225950>

    I'm not sure a user is going to understand what this (info level) log 
message means. Can you rewrite this from the perspective of what the user needs 
to know about what happened, or remove the log message?



geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderAdvisor.java
 (line 416)
<https://reviews.apache.org/r/53745/#comment225953>

    The way isPrimary is treated in this method is a bit confusing. Maybe is 
should actually be two different methods? Eg
    
    setIsPrimary() {
     sychronized(...) {
     isPrimary = true;
     
     notifyPrimaryLock();
     }
    }
    
    //this method just does the notification:
    notifyPrimaryLock() {
    ...
    }
    
    ...



geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderAdvisor.java
 (line 494)
<https://reviews.apache.org/r/53745/#comment225951>

    I think these checks probably belong in the while statement, above. 
Otherwise if the notify happens before this thread gets into the wait it will 
wait forever.


- Dan Smith


On Nov. 14, 2016, 10:28 p.m., xiaojian zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53745/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2016, 10:28 p.m.)
> 
> 
> Review request for geode, anilkumar gingade and Dan Smith.
> 
> 
> Bugs: geode-2107
>     https://issues.apache.org/jira/browse/geode-2107
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This is a thread leak we found in GEM-933.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/AbstractGatewaySenderEventProcessor.java
>  e953c0c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderAdvisor.java
>  ff36e06 
> 
> Diff: https://reviews.apache.org/r/53745/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> xiaojian zhou
> 
>

Reply via email to