[jira] [Commented] (CASSANDRA-8707) Move SegmentedFile, IndexSummary and BloomFilter to utilising RefCounted

["Benedict (JIRA)" <ji...@apache.org>]

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

Benedict commented on CASSANDRA-8707:
-------------------------------------

I've uploaded a patch [here|https://github.com/belliottsmith/cassandra/tree/8707-safercleanup]

There are a couple of minor semi-extraneous commits tweaking slightly the design just introduced in 7705, in particular:

* introducing a ref() method for those instances where we expect never to fail
* Letting Ref and RefCounted hold a reference to the thing they're counting, so that Ref.referent() can be supported

The patch itself then has some ground works:

* Encapsulate SSTableReader.readMeter, so it isn't accessed directly
* Introduce a new SharedCloseable interface that represents a Closeable that can be shared by creating a new instance via sharedCopy(); each instance can treated as distinct, but the underlying resources will only be closed once all of them are. it is backed by RefCounted, and just provides a convenient abstraction.
* Make IndexSummary, IFilter and SegmentedFile implement SharedCloseable

Finally we do the guts of the patch, which is refactoring the cleanup in SSTableReader so that instead of maintaining a chain of replacements, we have three tiers of resource management: instance-only, type-shared (TEMPLINK/FINAL), and global. Each have their own reference count managed to ensure safe cleanup. The instance-only fields include the major ones: dfile, ifile, summary and filter, as these are all managed directly via SharedCloseable. The type-shared and global manage general cleanup, such as dropping page cache, trashing read meter data, deleting files, etc.

Hopefully only the last bit is the only that requires considerable attention, and whilst it is a little verbose the logic should be much easier to understand the before, and hence much safer. It will also give us all of the new debug info if we get it wrong.

This patch exposes a number of new LEAK DETECTED warnings in unit tests which I want to work through, but I suspect mostly are down to creating and discarding sstables directly in the test methods. 

> Move SegmentedFile, IndexSummary and BloomFilter to utilising RefCounted
> ------------------------------------------------------------------------
>
>                 Key: CASSANDRA-8707
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8707
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Benedict
>            Assignee: Benedict
>             Fix For: 2.1.3
>
>
> There are still a few bugs with resource management, especially around SSTableReader cleanup, esp. when intermixing with compaction. This migration should help. We can simultaneously "simplify" the logic in SSTableReader to not track the replacement chain, only to take a new reference to each of the underlying resources.



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