cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefania (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-8897) Remove FileCacheService, instead pooling the buffers
Date Tue, 21 Apr 2015 10:49:59 GMT

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

Stefania edited comment on CASSANDRA-8897 at 4/21/15 10:49 AM:
---------------------------------------------------------------

Hi [~benedict] thank you for your review. I'm almost done with the updated version, I just
have a few questions:

- Are the logging gards something we always use for trace and debug or is there really a big
benefit when logging only with parametrized strings and no function calls?

- I did not understand your comment here:

{code}
    ByteBuffer page = chunk.get(BUFFER_SIZE);
    if (page.capacity() >= BUFFER_SIZE)
        buffers.add(page);
    else
        // this can occur under normal operation, if the chunk allocation is aligned, since
we will have
        // a single page spare at the end, and our microchunks are multiple pages
        logger.error("Discarding page with smaller capacity {}, expected {} bytes", page.capacity(),
BUFFER_SIZE);
{code}

How is this possible since {{Allocator.allocateAligned()}} limits the byte buffer to the exact
capacity requested,
which is an exact multiple of the microchunks size (BUFFER_SIZE). The existing utests do not
hit this code path, 
any suggestions on how to hit this code path?

- Can you elaborate a bit more on the CAS integer update part, at the end of this comment
here:

{code}
 // instead of any ugliness with getAddress() (which seems brittle, if we somehow grab the
wrong value or slice weirdly)
 // we can abuse the ByteBuffer internals; the internal "attachment" property in DBBs point
to the parent buffer of a slice
 // to ensure GC of the parent doesn't occur before the child, and hence free the native memory.
We don't need this,
 // but either way we can instead point to our Chunk object (which points to our Chunk ByteBuffer),
so that we can
 // directly lookup the Chunk we need to maintain. The main added advantage of this is that
we can also detect if
 // the chunk is not assigned to this thread, by assigning the Chunk the current local pool
on adoption, and confirming
 // we are operating on that exact pool now. ****We can then have a separate integer variable
we perform a CAS update on
 // only in this scenario, so that we can safely recover from the mistake****, and we log
some panic warnings. Or we can just
 // log the panic warnings for now.
{code}

Did you mean to have one thread unallocate the buffer from another thread pool or take over
the entire pool or other?


BTW, the '=' in allocateChunk() was definitely a mistake, thanks for spotting it and getting
rid of the rollback, which was ugly.



was (Author: stefania):
Hi [~benedict] thank you for your review. I'm almost done with the updated version, I just
have a few questions:

- Are the logging gards something we always use for trace and debug or is there really a big
benefit when logging only with parametrized strings and no function calls?

- I did not understand your comment here:

{code}
    ByteBuffer page = chunk.get(BUFFER_SIZE);
    if (page.capacity() >= BUFFER_SIZE)
        buffers.add(page);
    else
        // this can occur under normal operation, if the chunk allocation is aligned, since
we will have
        // a single page spare at the end, and our microchunks are multiple pages
        logger.error("Discarding page with smaller capacity {}, expected {} bytes", page.capacity(),
BUFFER_SIZE);
{code}

How is this possible since {Allocator.allocateAligned()} limits the byte buffer to the exact
capacity requested,
which is an exact multiple of the microchunks size (BUFFER_SIZE). The existing utests do not
hit this code path, 
any suggestions on how to hit this code path?

- Can you elaborate a bit more on the CAS integer update part, at the end of this comment
here:

{code}
 // instead of any ugliness with getAddress() (which seems brittle, if we somehow grab the
wrong value or slice weirdly)
 // we can abuse the ByteBuffer internals; the internal "attachment" property in DBBs point
to the parent buffer of a slice
 // to ensure GC of the parent doesn't occur before the child, and hence free the native memory.
We don't need this,
 // but either way we can instead point to our Chunk object (which points to our Chunk ByteBuffer),
so that we can
 // directly lookup the Chunk we need to maintain. The main added advantage of this is that
we can also detect if
 // the chunk is not assigned to this thread, by assigning the Chunk the current local pool
on adoption, and confirming
 // we are operating on that exact pool now. ****We can then have a separate integer variable
we perform a CAS update on
 // only in this scenario, so that we can safely recover from the mistake****, and we log
some panic warnings. Or we can just
 // log the panic warnings for now.
{code}

Did you mean to have one thread unallocate the buffer from another thread pool or take over
the entire pool or other?


BTW, the '=' in allocateChunk() was definitely a mistake, thanks for spotting it and getting
rid of the rollback, which was ugly.


> Remove FileCacheService, instead pooling the buffers
> ----------------------------------------------------
>
>                 Key: CASSANDRA-8897
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8897
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Benedict
>            Assignee: Stefania
>             Fix For: 3.0
>
>
> After CASSANDRA-8893, a RAR will be a very lightweight object and will not need caching,
so we can eliminate this cache entirely. Instead we should have a pool of buffers that are
page-aligned.



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

Mime
View raw message