You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jonathan Ellis (JIRA)" <ji...@apache.org> on 2012/11/10 06:17:12 UTC

[jira] [Created] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Jonathan Ellis created CASSANDRA-4942:
-----------------------------------------

             Summary: Pool [Compressed]RandomAccessReader on the partitioned read path
                 Key: CASSANDRA-4942
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
             Project: Cassandra
          Issue Type: Sub-task
          Components: Core
            Reporter: Jonathan Ellis
            Assignee: Jonathan Ellis
            Priority: Minor
             Fix For: 1.2.1




--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Resolved] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jonathan Ellis resolved CASSANDRA-4942.
---------------------------------------

    Resolution: Fixed

Fixed + committed.

Also fixed a tricky little problem with recyling a deallocated reader:

{code}
.       if (owner == null || buffer == null)
        {
            // The buffer == null check is so that if the pool owner has deallocated us, calling close()
            // will re-call deallocate rather than recycling a deallocated object.
            // I'd be more comfortable if deallocate didn't have to handle being idempotent like that,
            // but RandomAccessFile.close will call AbstractInterruptibleChannel.close which will
            // re-call RAF.close -- in this case, [C]RAR.close since we are overriding that.
            deallocate();
        }
        else
        {
            owner.recycle(this);
        }
{code}
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497140#comment-13497140 ] 

Sylvain Lebresne commented on CASSANDRA-4942:
---------------------------------------------

BufferedSegmentFile still don't pass the itself to the created RAR (and while I agree that this won't matter to a lot of people since mmap use is more widespread, I don't think there is any reason not to recycle (we'll still save the buffer allocation of RAR if we do)).

But with that fix, lgtm, +1.
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Pavel Yaskevich (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494568#comment-13494568 ] 

Pavel Yaskevich commented on CASSANDRA-4942:
--------------------------------------------

I don't think that the proposed change is much cleaner that what we have in CASSANDRA-4937 because now we add segmented file in the fix which is unnecessary. I will work on improving my implementation and will attach a patch to this issue. 
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494720#comment-13494720 ] 

Jonathan Ellis commented on CASSANDRA-4942:
-------------------------------------------

bq. it should be done transparently so we don't duplicate the code in getSegment per implementation

You're right, fixed + pushed to github.

bq. I don't like involving PooledSegmentedFile inside of RAR as well

Point taken, but since the proposed alternative is a global Map, I do think this is cleaner.  (You are right that Threadlocal makes cleanup too asynchronous, so I withdraw that suggestion.)
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Sylvain Lebresne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497110#comment-13497110 ] 

Sylvain Lebresne commented on CASSANDRA-4942:
---------------------------------------------

bq. SegmentedFile is the right place. It's basically a RAR factory

I agree, this also feels like the better place to me.

However, on the patch itself, either I'm missing something or it never gives the PoolingSegmentedFile to any RAR (and so won't pool/recycle anything).
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494565#comment-13494565 ] 

Jonathan Ellis commented on CASSANDRA-4942:
-------------------------------------------

The place that actually makes the most sense to do the pooling is the SegmentedFile implementation.  Patchset at http://github.com/jbellis/cassandra/branches/4942.
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497126#comment-13497126 ] 

Jonathan Ellis commented on CASSANDRA-4942:
-------------------------------------------

Fixed & pushed.
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494570#comment-13494570 ] 

Jonathan Ellis commented on CASSANDRA-4942:
-------------------------------------------

SegmentedFile is the right place.  It's basically a RAR factory.  This also gives us pooling of both CRAR and RAR for no extra complexity.
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CASSANDRA-4942) Pool [Compressed]RandomAccessReader on the partitioned read path

Posted by "Pavel Yaskevich (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-4942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13494575#comment-13494575 ] 

Pavel Yaskevich commented on CASSANDRA-4942:
--------------------------------------------

Let's agree to disagree. My idea of this is that it should be done transparently so we don't duplicate the code in getSegment per implementation, and I don't like involving PooledSegmentedFile inside of RAR as well. Caller shouldn't care what instance it gets when it calls open. I didn't bother doing caching for RAR because the default disk_access_node is mmap which is considered a good practice and keeping caching changes local to CRAR (which has the most overhead per open) allows to reuse caches basically for everything. 

If you think that this is the right way to go, +1.
                
> Pool [Compressed]RandomAccessReader on the partitioned read path
> ----------------------------------------------------------------
>
>                 Key: CASSANDRA-4942
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-4942
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Core
>            Reporter: Jonathan Ellis
>            Assignee: Jonathan Ellis
>            Priority: Minor
>             Fix For: 1.2.1
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira