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



src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java
<https://reviews.apache.org/r/15481/#comment55812>

    I think moving the assignment of closed = true into the if block is 
incorrect.
    
    Doing so allows a given client to decrement the client count multiple times 
on subsequent calls to close(), which is incorrect.
    
    It also allows continued access to a ZKI that has been closed so long as 
there are additional ZKIs around.
    
    In the event that we enter the InterruptedException handling block, we'll 
avoid setting this ZKI to closed wether the assignment is in the if block or 
outside of it.
    
    This probably needs to be handled in a follow-on jira so that the fix can 
apply to this backport and then forward to 1.6.


- Sean Busbey


On Nov. 13, 2013, 4:11 p.m., Sean Busbey wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15481/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 4:11 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1858
>     https://issues.apache.org/jira/browse/ACCUMULO-1858
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> ACCUMULO-1379 Fix edge cases if error in closing ZooKeeperInstance
> 
> (cherry picked from commit 3f6c66ede52cb1fb5a122d7bad06d7978ff0a671)
> 
> Reason: bugfix
> Author: Christopher Tubbs <[email protected]>
> Ref: ACCUMULO-1858
> 
> ACCUMULO-1379 - Adding close() to Instance to assist in freeing up resources
> 
> (cherry picked from commit 7da1164d87227960d3e0cfc841f753067e2c0304)
> 
> Reason: bugfix
> Author: John Vines <[email protected]>
> Ref: ACCUMULO-1858
> 
> 
> Diffs
> -----
> 
>   src/core/src/main/java/org/apache/accumulo/core/client/Instance.java 
> b3d09ba30b1175a495c9d0cb55b0b4a4f3aadbe1 
>   
> src/core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java 
> f657c07a28c3bf330556f0d4e02b9adcb0ea755e 
>   
> src/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftTransportPool.java
>  ef3724b149921b06cc27945eca11e4d0d1658c6b 
>   
> src/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java 
> 2ff7b8211de031a4fa04cae3c5bd7a3e03d50506 
>   src/core/src/main/java/org/apache/accumulo/core/util/ThriftUtil.java 
> 1b1cdd735bbf9772dc1a7f82e337125e675aa7b2 
>   src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooCache.java 
> f5bdd6b71652e8e2d664ca2d6480921f11663214 
>   src/core/src/main/java/org/apache/accumulo/core/zookeeper/ZooReader.java 
> 47663acbc421b890bc42bac2d7d5c5556119e6a8 
>   
> src/core/src/test/java/org/apache/accumulo/core/client/impl/TabletLocatorImplTest.java
>  538cb6c700423926cd1789654c2933a8ccbb1d65 
>   
> src/server/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java
>  e6cdb6397abe08883dc92700cbe2dc7ce7af6e1a 
> 
> Diff: https://reviews.apache.org/r/15481/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass. Functional tests in progress.
> 
> 
> Thanks,
> 
> Sean Busbey
> 
>

Reply via email to