[
https://issues.apache.org/jira/browse/CASSANDRA-7705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14289393#comment-14289393
]
Marcus Eriksson commented on CASSANDRA-7705:
--------------------------------------------
This LGTM for 2.1 inclusion - we have had so many issues related to the ref
counting lately we really need this
Comments;
* There is an AbstractRefCounted class in the patch which is not used, guess it
was replaced by doing the implementation in the RefCounted interface instead
* 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?
* Manager.extant - why a Map here? Could we use a Set?
* In RefState we directly access the CLQ in RefCountedState, could we
encapsulate this and give the methods name better names? Adding 'this' to a
'refs' collection does not tell me much when everything is called ref* :)
* In general, I think it would be nicer not putting all the classes inside the
RefCounted interface (atleast the public ones), breaking them out into their
own would make it a bit easier to follow (but that might just be my personal
preference)
* A few comments on the methods in RefCounted.* - especially Refs as it is the
most publicly visible
pushed a branch with a few nits fixed here:
https://github.com/krummas/cassandra/commits/bes/7705-2.1
> 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)
