cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benedict (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-3578) Multithreaded commitlog
Date Sun, 01 Dec 2013 11:22:39 GMT

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

Benedict edited comment on CASSANDRA-3578 at 12/1/13 11:20 AM:
---------------------------------------------------------------

bq. I understand what sync is doing
I just wanted to spell it all out so I could write next three times :-)

bq. Your own comments refer to "last" or "previous" in multiple places which speaks to how
difficult it is to avoid thinking of it that way

I agree it's difficult to avoid talking about last when talking about next, but this is typical
since it's chained. I still disagree about the rename, but as I alluded to do agree that both
are justifiable positions. To me it is very unintuitive in the sync() method (which is the
most complex method) to refer to the sync position we haven't yet populated as the "last"
sync marker position. Also, in the isFullySynced(), I think both work - as if the next as
past the end of the file, we're clearly referring to the next segment, no? I think we wouldn't
be having this argument if I'd stuck with "header" instead of "marker" :-)

That all said, it's definitely not worth any more argument, so I'll bow out at this point
if I still haven't persuaded you!

bq. couldn't we dispense with WaitQueue in favor of AtomicReference<Condition>
Well, WaitQueue is non-blocking, in a few senses:
# It is non-blocking in its *abstraction* which makes it absolutely the correct abstraction
to use here. To use a Condition object would require considerably more ugliness than a WaitQueue,
as we attempted to mimic its behaviour. However, you could implement a WaitQueue supporting
only signalAll() using a Lock and an AtomicReference<Condition> under the hood.
# It is non-blocking in its implementation (well, almost; certainly in this use it is non-blocking
with respect to consumers/producers blocking each other, but not necessarily consumers/consumers
or producers/producers, although this is easily rectified in its current design). In this
use case we could certainly handle blocking each other if we had to, but it is ugly to do
so when it isn't necessary in my book.
# When signalling threads, all waiting threads are activated immediately, whereas with a Condition
object they must wake up serially, incurring scheduler delay for each wake-up.

Something tangential also occurred to me, maybe worth spinning out a ticket for: As we currently
stand we can only support max_concurrent_write TOTAL writes per batch with BatchCLE. If we
were now to split out the CL.add() into the calling thread instead of the write stage, returning
a Future to wait on in the write stage, we could support max_connection writes instead, which
would be much larger. Although admittedly we would have to be careful about not OOMing if
there are a lot of large writes outstanding. It could do a lot for throughput in BatchCLE
potentially though.



was (Author: benedict):
bq. I understand what sync is doing
I just wanted to spell it all out so I could write next three times :-)

bq. Your own comments refer to "last" or "previous" in multiple places which speaks to how
difficult it is to avoid thinking of it that way

I agree it's difficult to avoid talking about last when talking about next, but this is typical
since it's chained. I still disagree about the rename, but as I alluded to do agree that both
are justifiable positions. To me it is very unintuitive in the sync() method (which is the
most complex method) to refer to the sync position we haven't yet populated as the "last"
sync marker position. Also, in the isFullySynced(), I think both work - as if the next as
past the end of the file, we're clearly referring to the next segment, no? I think we wouldn't
be having this argument if I'd stuck with "header" instead of "marker" :-)

That all said, it's definitely not worth any more argument, so I'll bow out at this point
if I still haven't persuaded you!

bq. couldn't we dispense with WaitQueue in favor of AtomicReference<Condition>
Well, WaitQueue is non-blocking, in two senses:
# It is non-blocking in its *abstraction* which makes it absolutely the correct abstraction
to use here. To use a Condition object would require considerably more ugliness than a WaitQueue,
as we attempted to mimic its behaviour. However, you could implement a WaitQueue supporting
only signalAll() using a Lock and an AtomicReference<Condition> under the hood.
# It is non-blocking in its implementation (well, almost; certainly in this use it is non-blocking
with respect to consumers/producers blocking each other, but not necessarily consumers/consumers
or producers/producers, although this is easily rectified in its current design). In this
use case we could certainly handle blocking each other if we had to, but it is ugly to do
so when it isn't necessary in my book.



> Multithreaded commitlog
> -----------------------
>
>                 Key: CASSANDRA-3578
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3578
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jonathan Ellis
>            Assignee: Benedict
>            Priority: Minor
>              Labels: performance
>         Attachments: 0001-CASSANDRA-3578.patch, ComitlogStress.java, Current-CL.png,
Multi-Threded-CL.png, TestEA.java, latency.svg, oprate.svg, parallel_commit_log_2.patch
>
>
> Brian Aker pointed out a while ago that allowing multiple threads to modify the commitlog
simultaneously (reserving space for each with a CAS first, the way we do in the SlabAllocator.Region.allocate)
can improve performance, since you're not bottlenecking on a single thread to do all the copying
and CRC computation.
> Now that we use mmap'd CommitLog segments (CASSANDRA-3411) this becomes doable.
> (moved from CASSANDRA-622, which was getting a bit muddled.)



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Mime
View raw message