[jira] [Comment Edited] (CASSANDRA-14248) SSTableIndex should not use Ref#globalCount() to determine when to delete index file

["Jordan West (Jira)" <ji...@apache.org>]

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

Jordan West edited comment on CASSANDRA-14248 at 5/4/20, 8:19 PM:
------------------------------------------------------------------

[~jbellis] thanks for the updated link. I will take a look. Do you have a test that validates the bit about {{discoverForComponents}}. I took one of the existing \{{SASIIndexTest}}s and added the following:

 
{code:java}
        if (forceFlush)
        {
            for (SSTable sst: store.getLiveSSTables())
            {
                Set<Component> components = SSTable.discoverComponentsFor(sst.descriptor);
                System.out.println("Components: " + components);
            }        
       }
{code}
When I run it, I see {{Components: [Data.db, Index.db, Filter.db, Digest.crc32, Statistics.db, TOC.txt, Summary.db, CompressionInfo.db]}}. 

 

Reading {{discoverComponentsFor}}, my take is that {{Descriptor#filenameFor(Component)}} doesn't properly build the index file name (since it doesn't have the index name itself and Component.Type.SECONDARY_INDEX.repr = "SI_*.db" which is not actually doing glob matching). This results in the exist call failing and the indexes being excluded. 


was (Author: jrwest):
[~jbellis] thanks for the updated link. I will take a look. Do you have a test that validates the bit about {{discoverForComponents}}. I took one of the existing \{{SASIIndexTest}}s and added the following:

 
{code:java}
        if (forceFlush)
        {
            for (SSTable sst: store.getLiveSSTables())
            {
                Set<Component> components = SSTable.discoverComponentsFor(sst.descriptor);
                System.out.println("Components: " + components);
            }        }
{code}
When I run it, I see {{Components: [Data.db, Index.db, Filter.db, Digest.crc32, Statistics.db, TOC.txt, Summary.db, CompressionInfo.db]}}. 

 

Reading {{discoverComponentsFor}}, my take is that {{Descriptor#filenameFor(Component)}} doesn't properly build the index file name (since it doesn't have the index name itself and Component.Type.SECONDARY_INDEX.repr = "SI_*.db" which is not actually doing glob matching). 

> SSTableIndex should not use Ref#globalCount() to determine when to delete index file
> ------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-14248
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14248
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Feature/SASI
>            Reporter: Jordan West
>            Assignee: Jordan West
>            Priority: Normal
>             Fix For: 3.11.x
>
>
> {{SSTableIndex}} instances maintain a {{Ref}} to the underlying {{SSTableReader}} instance. When determining whether or not to delete the file after the last {{SSTableIndex}} reference is released, the implementation uses {{sstableRef.globalCount()}}: [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/index/sasi/SSTableIndex.java#L135.] This is incorrect because {{sstableRef.globalCount()}} returns the number of references to the specific instance of {{SSTableReader}}. However, in cases like index summary redistribution, there can be more than one instance of {{SSTableReader}}. Further, since the reader is shared across multiple indexes, not all indexes see the count go to 0. This can lead to cases where the {{SSTableIndex}} file is incorrectly deleted or not deleted when it should be.
>  
> A more correct implementation would be to either:
>  * Tie into the existing {{SSTableTidier}}. SASI indexes already are SSTable components but are not cleaned up by the {{SSTableTidier}} because they are not found with the currently cleanup implementation
>  * Revamp {{SSTableIndex}} reference counting to use {{Ref}} and implement a new tidier. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org