Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Tue, 27 Jan 2015 23:20:35 +0000 (UTC)
From: "Benedict (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12732298.1407330950000.189455.1422400835254@Atlassian.JIRA>
In-Reply-To: <JIRA.12732298.1407330950000@Atlassian.JIRA>
References: <JIRA.12732298.1407330950000@Atlassian.JIRA>
 <JIRA.12732298.1407330950482@arcas>
Subject: [jira] [Commented] (CASSANDRA-7705) Safer Resource Management
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Benedict commented on CASSANDRA-7705:
-------------------------------------

I've uploaded a further polished version [here|https://github.com/belliottsmith/cassandra/commits/7705-2.1-x], hopefully addressing all of your nits and concerns, and finishing up your ref -> tryRef refactor.

Responding to a couple of the more specific points that I haven't changed:

bq. In SSTableLoader the Ref.sharedRef() is passed to the constructor in SSTableStreamingSections which feels wrong, shouldn't we acquire a new Ref and have SSTSS release that once it is done with the sstable?

I didn't want to dive too closely into each prior design decision in this patch, I just wanted to replicate the existing behaviour. This is an offline operation, so I'm not sure it matters or not, but this is the prior behaviour.

bq. Manager.extant - why a Map here? Could we use a Set?

There is no ConcurrentHashSet in the JDK, and I don't want to use some random one. NBHS can leave dangling references to objects lying around after removal IIRC, and we don't need the performance here, so CHM it was.


> Safer Resource Management
> -------------------------
>
>                 Key: CASSANDRA-7705
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7705
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Benedict
>            Assignee: Benedict
>             Fix For: 3.0
>
>
> We've had a spate of bugs recently with bad reference counting. these can have potentially dire consequences, generally either randomly deleting data or giving us infinite loops. 
> Since in 2.1 we only reference count resources that are relatively expensive and infrequently managed (or in places where this safety is probably not as necessary, e.g. SerializingCache), we could without any negative consequences (and only slight code complexity) introduce a safer resource management scheme for these more expensive/infrequent actions.
> Basically, I propose when we want to acquire a resource we allocate an object that manages the reference. This can only be released once; if it is released twice, we fail immediately at the second release, reporting where the bug is (rather than letting it continue fine until the next correct release corrupts the count). The reference counter remains the same, but we obtain guarantees that the reference count itself is never badly maintained, although code using it could mistakenly release its own handle early (typically this is only an issue when cleaning up after a failure, in which case under the new scheme this would be an innocuous error)


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)