Mailing-List: contact notifications-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jira@apache.org
Date: Wed, 13 Nov 2013 21:15:22 +0000 (UTC)
From: "Christopher Tubbs (JIRA)" <jira@apache.org>
To: notifications@accumulo.apache.org
Message-ID: <JIRA.12679053.1384361629449.69270.1384377322023@arcas>
In-Reply-To: <JIRA.12679053.1384361629449@arcas>
References: <JIRA.12679053.1384361629449@arcas>
Subject: [jira] [Commented] (ACCUMULO-1889) ZooKeeperInstance close method
 should mark instance closed.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

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

I don't like catching Exception, so I'd prefer a separate block that increments and re-throws. Exception isn't expressive enough about which exceptions the code is resilient against (one of the benefits of bumping to JDK 1.7 is reusing block, without losing expressiveness). Besides, "Exception"-only runs the risk of somebody deleting the entire block when the InterruptedException is handled better, without realizing that AccumuloException still needs to be handled before being re-thrown.

> 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.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)