You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2016/05/04 18:05:06 UTC

Re: Review Request 46275: GEODE-1234: Provide a test hook to track transactions scheduled to be removed

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




geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ClientHealthMonitor.java (line 282)
<https://reviews.apache.org/r/46275/#comment195752>

    Instead of HashSet use ConcurrentHashSet so you don't have concurrency issues since multiple threads can add/remove from this.
    
    I don't see why this is "static".
    I think it makes more sense for "scheduledToBeRemovedTx" to be a non-static private instance variable that is initialized to null if the hook is not enabled and other wise initialized to an empty ConcurrentHashSet.



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ClientHealthMonitor.java (line 314)
<https://reviews.apache.org/r/46275/#comment195753>

    I would like to see this entire testHook only do the addAll and removeAll (and the allocation of the  HashSet) if the hook has been enabled. Don't do this code in production. Test's that use the hook can enable it somehow (could be a system property) otherwise the set can be null.


- Darrel Schneider


On April 15, 2016, 10:25 a.m., Eric Shu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46275/
> -----------------------------------------------------------
> 
> (Updated April 15, 2016, 10:25 a.m.)
> 
> 
> Review request for geode, Darrel Schneider and Swapnil Bawaskar.
> 
> 
> Bugs: GEOD-1234
>     https://issues.apache.org/jira/browse/GEOD-1234
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Provide a test hook to track transactions to be removed instead of waiting for default 180 seconds.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/ClientHealthMonitor.java 93e543d 
> 
> Diff: https://reviews.apache.org/r/46275/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eric Shu
> 
>