[
https://issues.apache.org/jira/browse/CASSANDRA-8707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14303099#comment-14303099
]
Benedict edited comment on CASSANDRA-8707 at 2/3/15 12:21 PM:
--------------------------------------------------------------
bq. SSTableDeletingTask - why not notify when deleting tmplink files?
We never did before - in fact we never called this for tmplink files
previously, but since they could fail to delete for whatever reason, it seemed
sensible to make it be called for those whilst cleaning this up. I'm assuming
listeners to deletion notifications care about _actual_ deletion, as opposed to
deletion of an interim file, so I expect the prior behaviour is correct.
bq. Memory - looks like a fix for some other issue included, break that out in
a new ticket?
Split out into CASSANDRA-8726.
bq. BloomFilter - the protected copy constructor, why not use the hashCount and
bitset from the copy that is passed in?
Good point!
bq. In general I think the new class hierarchy inside SSTableReader
(TypeManager/GlobalManager/TidyManager/TypeTidy/InstanceTidier) needs to be
simplified and documented (basically no comments at all right now). I find it
very hard to follow, perhaps it is because of the naming of the classes (for
example, the name "TypeManager" is quite generic and tells me very little) or
that they are nested and access each others private fields.
OK, I'll see what I can do to comment that better. The minimal comments were
"One Per X" which perhaps weren't very clear - they are expanding umbrellas of
shared state: instance, descriptor "type", and global ("per logical sstable").
The XManager is just a shared abstraction for tracking the umbrellas, so when a
new reader is constructed it asks a manager for a reference to its shared
state. I'll see if I can make this much clearer.
was (Author: benedict):
bq. SSTableDeletingTask - why not notify when deleting tmplink files?
We never did before - in fact we never called this for tmplink files
previously, but since they could fail to delete for whatever reason, it seemed
sensible to make it be called for those whilst cleaning this up. I'm assuming
listeners to deletion notifications care about _actual_ deletion, as opposed to
deletion of an interim file, so I expect the prior behaviour is correct.
bq. Memory - looks like a fix for some other issue included, break that out in
a new ticket?
Not so much a fix, as simply throwing in an assertion error if we fail to
allocate. It's an oversight that _has_ been seen as a possible problem in a
couple of places recently. Happy to split it out if you prefer, but it seemed
innoccuous with the other changes.
bq. BloomFilter - the protected copy constructor, why not use the hashCount and
bitset from the copy that is passed in?
Good point!
bq. In general I think the new class hierarchy inside SSTableReader
(TypeManager/GlobalManager/TidyManager/TypeTidy/InstanceTidier) needs to be
simplified and documented (basically no comments at all right now). I find it
very hard to follow, perhaps it is because of the naming of the classes (for
example, the name "TypeManager" is quite generic and tells me very little) or
that they are nested and access each others private fields.
OK, I'll see what I can do to comment that better. The minimal comments were
"One Per X" which perhaps weren't very clear - they are expanding umbrellas of
shared state: instance, descriptor "type", and global ("per logical sstable").
The XManager is just a shared abstraction for tracking the umbrellas, so when a
new reader is constructed it asks a manager for a reference to its shared
state. I'll see if I can make this much clearer.
> 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)
