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

Marcus Eriksson commented on CASSANDRA-6689:
--------------------------------------------

Overall this is a very complicated/involved change that we really need to 
test/benchmark (I can't make it go faster than HeapSlabPool)/figure out if we 
want it in. That being said, it looks very solid.

The biggest risks are, I guess (and where I spent most of my time when 
reviewing), that we start corrupting/freeing used/leaking offheap memory, but 
as far as I can tell, the patch avoids that. We should do long-running tests to 
verify it as well.

It would probably have been nice to split this patch up in smaller pieces, say 
doing the GC in a separate ticket and only discarding entire regions on flush 
here, but I understand it would be difficult to separate the things.

A commit on top of this pushed to 
https://github.com/krummas/cassandra/commits/marcuse/6689-3 with comment 
fixups/small changes.

* GC metrics! Time spent, amount of data reallocated etc.
* In NBQV, drainTo does not return the correct number of inserted items.
* Class structure in WaitQueue a bit confusing (mixing static/non-static inner 
classes, signalledUpdater in WaitQueue while updating field in 
RegisteredSignal. Small refactor in my branch, feel free to ignore if you 
prefer your way.
* Should we add QueryProcessor.process(<without RefAction>) methods to avoid 
all the RefAction.allocateOnHeap() methods? Or, perhaps it is good to always 
force people to think about where they want the data.
* Config file wrong, repeated memtable_cleanup_threshold
* Passing null as RefAction to Rows constructor (ListPermissionsStatement and 
Rows.subcodec
* Make OpOrder.Group and RefAction implement AutoClosable to be able to do 
try-with-resources?
* AtomicReferenceArrayUpdater.shift(), really? :)
* CommitState unused
* Would be nice if we didn't have to copy data to heap for thrift queries, but 
i guess that is out of scope for now.

-- silly single-node perf testing (on my laptop): --
* Seems we get about 10% less write op rate throughput with OffHeapPool.
* I flush ~40MB/s with HeapSlabPool but only ~15MB/s with OffHeapPool.
* Running stress leads to client TimedOutException during flushing (I guess due 
to the slower flushing)

Oh, and a rebase on 2.1 would be nice, does not work running in a cluster now.

Also, just throwing the idea out here, maybe we should make this a separate 
library? It would make fixing bugs etc in it much more painful, but with a bit 
of thinking it could be useful for external users. Probably not possible for a 
2.1 timeframe, but maybe in the future.

I'll spend some more time on this, running actual tests etc.


> Partially Off Heap Memtables
> ----------------------------
>
>                 Key: CASSANDRA-6689
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-6689
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Core
>            Reporter: Benedict
>            Assignee: Benedict
>             Fix For: 2.1 beta2
>
>
> Move the contents of ByteBuffers off-heap for records written to a memtable.
> (See comments for details)



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to