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

Ariel Weisberg commented on CASSANDRA-8670:
-------------------------------------------

In what scenarios do we end up actually writing to the native methods right 
now? In most cases we opt to use the channel when available. The only time we 
should be using WrappedDataOutputStreamPlus is when we aren't actually writing 
to the channel of a file or socket.

Where we could really do better is when using compression, we could have the 
compression wrap a direct buffer which wraps the channel and avoid relying on 
the built in mechanisms for getting data off heap.

I had some other refrigerator moments overnight as well.

BufferedDataOutputStreamPlus.close() will clean the buffer even if it might not 
own the buffer such as when it is provided to the constructor. It also doesn't 
check if the channel is null.

When used from the commit log without a channel it will throw an NPE if it 
exceeds the capacity of the buffer when it goes to flush. I suppose one runtime 
exception is as good as another. There is also the extra bounds check in 
ensureRemaining() which always seemed a little useless to me and the fact that 
it will not drop through and do efficient copies for direct byte buffers. It 
almost seems like there is a case for a version just for wrapping fixed size 
bytebuffers. 

> Large columns + NIO memory pooling causes excessive direct memory usage
> -----------------------------------------------------------------------
>
>                 Key: CASSANDRA-8670
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8670
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Ariel Weisberg
>            Assignee: Ariel Weisberg
>             Fix For: 3.0
>
>         Attachments: largecolumn_test.py
>
>
> If you provide a large byte array to NIO and ask it to populate the byte 
> array from a socket it will allocate a thread local byte buffer that is the 
> size of the requested read no matter how large it is. Old IO wraps new IO for 
> sockets (but not files) so old IO is effected as well.
> Even If you are using Buffered{Input | Output}Stream you can end up passing a 
> large byte array to NIO. The byte array read method will pass the array to 
> NIO directly if it is larger than the internal buffer.  
> Passing large cells between nodes as part of intra-cluster messaging can 
> cause the NIO pooled buffers to quickly reach a high watermark and stay 
> there. This ends up costing 2x the largest cell size because there is a 
> buffer for input and output since they are different threads. This is further 
> multiplied by the number of nodes in the cluster - 1 since each has a 
> dedicated thread pair with separate thread locals.
> Anecdotally it appears that the cost is doubled beyond that although it isn't 
> clear why. Possibly the control connections or possibly there is some way in 
> which multiple 
> Need a workload in CI that tests the advertised limits of cells on a cluster. 
> It would be reasonable to ratchet down the max direct memory for the test to 
> trigger failures if a memory pooling issue is introduced. I don't think we 
> need to test concurrently pulling in a lot of them, but it should at least 
> work serially.
> The obvious fix to address this issue would be to read in smaller chunks when 
> dealing with large values. I think small should still be relatively large (4 
> megabytes) so that code that is reading from a disk can amortize the cost of 
> a seek. It can be hard to tell what the underlying thing being read from is 
> going to be in some of the contexts where we might choose to implement 
> switching to reading chunks.



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

Reply via email to