accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christopher Tubbs (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ACCUMULO-1889) ZooKeeperInstance close method should mark instance closed.
Date Fri, 15 Nov 2013 01:59:23 GMT

    [ https://issues.apache.org/jira/browse/ACCUMULO-1889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13823204#comment-13823204
] 

Christopher Tubbs commented on ACCUMULO-1889:
---------------------------------------------

Ugh. I hadn't realized how bad the InterruptedException got propagated throughout our interfaces,
and I didn't realize that AccumuloException wasn't being thrown from the try block (only explicitly
thrown in the catch block).

It seems to me that the best fix is to:
# patch org.apache.accumulo.fate.zookeeper.ZooReader.close() to handle the ZooKeeper close()'s
InterruptedException properly with {{Thread.currentThread().interrupt()}}
# stop propagating InterruptedException through the hierarchy of close() statements
# drop the AccumuloException from the Instance.close() method signature and make Interface
extend java.io.Closeable instead (at the very least, drop the unnecessary AccumuloException
and make it AutoCloseable when we switch to JDK1.7; changing it to java.io.Closeable now would
ease that)
# change that catch statement to catch RuntimeExceptions instead of InterruptedException and
rethrow them after fixing the counter


> ZooKeeperInstance close method should mark instance closed.
> -----------------------------------------------------------
>
>                 Key: ACCUMULO-1889
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-1889
>             Project: Accumulo
>          Issue Type: Bug
>    Affects Versions: 1.4.5, 1.5.1, 1.6.0
>            Reporter: Sean Busbey
>            Assignee: Sean Busbey
>             Fix For: 1.4.5, 1.5.1, 1.6.0
>
>         Attachments: ACCUMULO-1889.1.patch.txt
>
>
> (1.4.5 and 1.5.1 impact presumes ACCUMULO-1858 gets applied)
> The current close() implementation on ZooKeeperInstance only marks a given instance as
closed if the outstanding client count is 0.
> {code}
>   public synchronized void close() throws AccumuloException {
>     if (!closed && clientInstances.decrementAndGet() == 0) {
>       try {
>         zooCache.close();
>         ThriftUtil.close();
>       } catch (InterruptedException e) {
>         clientInstances.incrementAndGet();
>         throw new AccumuloException("Issues closing ZooKeeper.");
>       }
>       closed = true;
>     }
>   }
> {code}
> This is incorrect for two reason:
> 1) It allows continued operations on a given ZKI after it has had close() called on it
> 2) It allows a given ZKI to decrement the number of open clients more than once



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Mime
View raw message