cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "graham sanderson (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-7546) AtomicSortedColumns.addAllWithSizeDelta has a spin loop that allocates memory
Date Fri, 10 Oct 2014 22:02:35 GMT

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

graham sanderson edited comment on CASSANDRA-7546 at 10/10/14 10:02 PM:
------------------------------------------------------------------------

Sorry [~yukim], I somehow missed your update - I'm about to attach the test results here...
note they show much higher GC issues in native_obj than heap_buffers without the fix, I'm
guessing because the spinning is much faster with native_obj

As for monitorEnter/monitorExit Benedict and I had a discussion about that above (I originally
had it with either multiple copies of the code, or nested functions), but it complicated stuff,
and I was unable to prove any issues with monitorEnter or monitorExit (or indeed reference
any, other than some vague suspicions I had that maybe this excludes biased locking  or anything
else which assumes these are neatly paired in a stack frame). In any case we don't really
care because if we are using them we've already proved we're contended, and the monitor would
be inflated anyway. The other issue was the use of Unsafe, but Benedict seemed fine with that
also, since without Unsafe (which most people have) you just get the old behavior

So, I say go ahead and promote the fix as is (yes current 2.1 trunk seemed to have Locks.java
already added - I didn't diff them, but I peeked briefly and it looked about the same)

It is possible someone will find a usage scenario that this makes slower, in which case we
can look at that, but I suspect as mentioned before, in all of these cases where we degrade
performance it is probably because the original performance is just on a lucky knife edge
between under utilization, and a complete mess!

Finally, I'll summarize what Benedict said up above, that whilst we could add a switch for
this, this is really an internal implementation fix, the goal of which is eventually that
there should be no bottleneck even when mutating the same partition (something he planned
to address in version >=3.0 with lazy updates, and repair on read)


was (Author: graham sanderson):
Sorry [~yukim], I somehow missed your update - I'm about to attach the test results here...
note they show much higher GC issues in native_obj than heap_buffers without the fix, I'm
guessing because the spinning is much faster with native_obj

As for monitorEnter/monitorExit Benedict and I had a discussion about that above (I originally
had it with either multiple copies of the code, or nested functions), but it complicated stuff,
and I was unable to prove any issues with monitorEnter or monitorExit (or indeed reference
any, other than some vague suspicions I had that maybe this excludes biased locking  or anything
else which assumes these are neatly paired in a stack frame). In any case we don't really
care because if we are using them we've already proved we're contended, and the monitor would
be inflated anyway. The other issue was the use of Unsafe, but Benedict seemed fine with that
also, since without Unsafe (which most people have) you just get the old behavior

So, I say go ahead and promote the fix as is (yes current 2.1 trunk seemed to have Locks.java
already added - I didn't diff them, but I peeked briefly and it looked about the same)

It is possible someone will find a usage scenario that this makes slower, in which case we
can look at that, but I suspect as mentioned before, in all of these cases where we degrade
performance it is probably because the original performance is just on a lucky knife edge
between under utilization, and a complete mess!

Finally, I'll summarize what Benedict said up above, that whilst we could add a switch for
this, this is really an internal implementation fix, the goal of which is eventually that
there should be no bottleneck even when mutation the same partition (something he planned
to address in version >=3.0 with lazy updates, and repair on read)

> AtomicSortedColumns.addAllWithSizeDelta has a spin loop that allocates memory
> -----------------------------------------------------------------------------
>
>                 Key: CASSANDRA-7546
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-7546
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: graham sanderson
>            Assignee: graham sanderson
>             Fix For: 2.1.1
>
>         Attachments: 7546.20.txt, 7546.20_2.txt, 7546.20_3.txt, 7546.20_4.txt, 7546.20_5.txt,
7546.20_6.txt, 7546.20_7.txt, 7546.20_7b.txt, 7546.20_alt.txt, 7546.20_async.txt, 7546.21_v1.txt,
cassandra-2.1-7546-v2.txt, cassandra-2.1-7546.txt, graph2_7546.png, graph3_7546.png, graph4_7546.png,
graphs1.png, hint_spikes.png, suggestion1.txt, suggestion1_21.txt, young_gen_gc.png
>
>
> In order to preserve atomicity, this code attempts to read, clone/update, then CAS the
state of the partition.
> Under heavy contention for updating a single partition this can cause some fairly staggering
memory growth (the more cores on your machine the worst it gets).
> Whilst many usage patterns don't do highly concurrent updates to the same partition,
hinting today, does, and in this case wild (order(s) of magnitude more than expected) memory
allocation rates can be seen (especially when the updates being hinted are small updates to
different partitions which can happen very fast on their own) - see CASSANDRA-7545
> It would be best to eliminate/reduce/limit the spinning memory allocation whilst not
slowing down the very common un-contended case.



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

Mime
View raw message