[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[ https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sylvain Lebresne updated CASSANDRA-5699: Attachment: (was: 5699.txt) > Streaming (2.0) can deadlock > > > Key: CASSANDRA-5699 > URL: https://issues.apache.org/jira/browse/CASSANDRA-5699 > Project: Cassandra > Issue Type: Bug >Reporter: Sylvain Lebresne >Assignee: Sylvain Lebresne > Fix For: 2.0 beta 1 > > Attachments: 5699.txt > > > The new streaming implementation (CASSANDRA-5286) creates 2 threads per host > for streaming, one for the incoming stream and one for the outgoing one. > However, both currently share the same socket, but since we use synchronous > I/O, a read can block a write, which can result in a deadlock if 2 nodes are > both blocking on a read a the same time, thus blocking their respective > writes (this is actually fairly easy to reproduce with a simple repair). > So instead attaching a patch that uses one socket per thread. > The patch also correct the stream throughput throttling calculation that was > 8000 times lower than what it should be. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[
https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sylvain Lebresne updated CASSANDRA-5699:
Attachment: 5699.txt
bq. It's somewhat confusing that we use StreamInit both as leader and as
follower, to mean different things
I guess, though if you see StreamInit as a way for both node to open/initialize
their outgoing connection to the other node (which is what this is doing), it's
not really that different. And it's somewhat consistent with the rest of the
protocol, where each side will sent a Prepare and then finally a Complete.
Anyway, updated the patch with the sentByInitiator renaming.
bq. What is going on with the switch from Set ongoingSessions to
Map ongoingSessions in SRF?
ongoingSession only role initialy was to track how many sessions for a
StreamResultFuture were live to know when we're done (SRF.maybeComplete()). In
the original patch of CASSANDRA-5286, it wasn't even there, and there was just
an AtomicInteger decremented each time a session completed. I was slightly
afraid that a race could have us counting the same session done twice, so I
changed it to a Set and added a simple UUID per session that was only
used for that purpose (it was never send to the other side or anything).
Anyway, that UUID addition was stupid in the first place since for a SRF, there
is only one StreamSession per remote host, so it should have been a
Set to start with and we have no use for a StreamSession UUID.
But on top of that, this patch add the need to be able to find back a
StreamSession in a SRF given the remote endpoint (in IncomingStreamConnection,
when the initiator gets the other side StreamInit), so we really need a map now.
bq. Suggest renaming, e.g. onSessionEstablished
Agreed, though I've renamed to onInitializationComplete to be consistent with
the wording of the javadoc.
bq. Somewhat confused by logic in complete() – "I received a Complete message.
If I'm already waiting for a complete message, close the session. Otherwise,
wait for [another] Complete message" ?
Each side will send a CompleteMessage to say "I'm done on my side", and the
session will be considered really complete (and closed) when we got the
Complete for both side. So WAIT_COMPLETE means "I've seen only one complete
message so far". Hence the logic of complete() is "I just received a complete
from the other side, if I had already completed my side, close the session,
otherwise move to the "I'm waiting for the other complete" state".
bq. It looks like there may be synchronization issues with "state;"
I think we're good, because the only case where we can race to set a state is
during the completion phase, for WAIT_COMPLETE and COMPLETE, and those are
protected. For other states, there are set sequentially on each node by
construction.
bq. Why is init broken out from construction?
SRF takes his StreamSession in his ctor so we can't have StreamSessions take
their SRF without some other refactor. Now don't get me wrong, it bugs me too.
And I do think there is a few things we can do to make that whole new streaming
API simpler/cleaner (including probably renaming StreamRepairFuture). And I'm
volonteering to give to a shot to that. But in another follow ticket because:
# I really think that kind of code cleanup shouldn't block beta1 (but this
ticket, the fact that streaming deadlock, do is a blocker)
# I've already rewrote quite a bit of Yuki's code without him having the change
to chime in. Would feel fair to wait to him to be back before refactoring parts
that are not really crucial for beta1.
bq. Why do we include a String description in SIM?
This the description of what the operation is doing ("Repair", "Bootstrap",
"Restore replica count", ...)
bq. When does follower immediately have something to stream to leader
A StreamSession in the initiator handle both incoming and outgoing streams.
Sometimes we only have outgoing ones (Unbootstrap/Restore replica
count/Bulkloading), sometimes only
incoming ones (Bootstrap) and sometimes both (Repair indeed but also the
SS.RangeRelocator for moves and vnodes tokens relocation). Does that answer
your question?
> Streaming (2.0) can deadlock
>
>
> Key: CASSANDRA-5699
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5699
> Project: Cassandra
> Issue Type: Bug
>Reporter: Sylvain Lebresne
>Assignee: Sylvain Lebresne
> Fix For: 2.0 beta 1
>
> Attachments: 5699.txt
>
>
> The new streaming implementation (CASSANDRA-5286) creates 2 threads per host
> for streaming, one for the incoming stream and one for the outgoing one.
> However, both currently share the same socket, but since we use synchronous
> I/O, a read can block a write, which can result in a deadlock if 2 nodes are
[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[ https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sylvain Lebresne updated CASSANDRA-5699: Attachment: 5699.txt bq. Is the "stream lifecycle" documented anywhere Yuki had written https://gist.github.com/yukim/5672508. The one thing this patch changes compared to that "design document" is that the initialization phase is slightly more complex since we need to create 2 connection. So the first node sends a StreamInit to the other end to create the first connection (as was done previously), but then the remote side creates a connection back sending a StreamInit message of it's own. Then and only then do we go to the prepare phase. In any case, we probably want that lifecycle to be documented in the javadoc or we'll lose track of it, so I've described this in relative detail at the head of StreamSession. So updated the patch with those added comments and the modification to StreamRepairTask cleaned up. > Streaming (2.0) can deadlock > > > Key: CASSANDRA-5699 > URL: https://issues.apache.org/jira/browse/CASSANDRA-5699 > Project: Cassandra > Issue Type: Bug >Reporter: Sylvain Lebresne >Assignee: Sylvain Lebresne > Fix For: 2.0 beta 1 > > Attachments: 5699.txt > > > The new streaming implementation (CASSANDRA-5286) creates 2 threads per host > for streaming, one for the incoming stream and one for the outgoing one. > However, both currently share the same socket, but since we use synchronous > I/O, a read can block a write, which can result in a deadlock if 2 nodes are > both blocking on a read a the same time, thus blocking their respective > writes (this is actually fairly easy to reproduce with a simple repair). > So instead attaching a patch that uses one socket per thread. > The patch also correct the stream throughput throttling calculation that was > 8000 times lower than what it should be. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[ https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sylvain Lebresne updated CASSANDRA-5699: Attachment: (was: 5699.txt) > Streaming (2.0) can deadlock > > > Key: CASSANDRA-5699 > URL: https://issues.apache.org/jira/browse/CASSANDRA-5699 > Project: Cassandra > Issue Type: Bug >Reporter: Sylvain Lebresne >Assignee: Sylvain Lebresne > Fix For: 2.0 beta 1 > > > The new streaming implementation (CASSANDRA-5286) creates 2 threads per host > for streaming, one for the incoming stream and one for the outgoing one. > However, both currently share the same socket, but since we use synchronous > I/O, a read can block a write, which can result in a deadlock if 2 nodes are > both blocking on a read a the same time, thus blocking their respective > writes (this is actually fairly easy to reproduce with a simple repair). > So instead attaching a patch that uses one socket per thread. > The patch also correct the stream throughput throttling calculation that was > 8000 times lower than what it should be. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[ https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jonathan Ellis updated CASSANDRA-5699: -- Reviewer: jbellis > Streaming (2.0) can deadlock > > > Key: CASSANDRA-5699 > URL: https://issues.apache.org/jira/browse/CASSANDRA-5699 > Project: Cassandra > Issue Type: Bug >Reporter: Sylvain Lebresne >Assignee: Sylvain Lebresne > Fix For: 2.0 beta 1 > > Attachments: 5699.txt > > > The new streaming implementation (CASSANDRA-5286) creates 2 threads per host > for streaming, one for the incoming stream and one for the outgoing one. > However, both currently share the same socket, but since we use synchronous > I/O, a read can block a write, which can result in a deadlock if 2 nodes are > both blocking on a read a the same time, thus blocking their respective > writes (this is actually fairly easy to reproduce with a simple repair). > So instead attaching a patch that uses one socket per thread. > The patch also correct the stream throughput throttling calculation that was > 8000 times lower than what it should be. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CASSANDRA-5699) Streaming (2.0) can deadlock
[ https://issues.apache.org/jira/browse/CASSANDRA-5699?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sylvain Lebresne updated CASSANDRA-5699: Attachment: 5699.txt > Streaming (2.0) can deadlock > > > Key: CASSANDRA-5699 > URL: https://issues.apache.org/jira/browse/CASSANDRA-5699 > Project: Cassandra > Issue Type: Bug >Reporter: Sylvain Lebresne >Assignee: Sylvain Lebresne > Fix For: 2.0 beta 1 > > Attachments: 5699.txt > > > The new streaming implementation (CASSANDRA-5286) creates 2 threads per host > for streaming, one for the incoming stream and one for the outgoing one. > However, both currently share the same socket, but since we use synchronous > I/O, a read can block a write, which can result in a deadlock if 2 nodes are > both blocking on a read a the same time, thus blocking their respective > writes (this is actually fairly easy to reproduce with a simple repair). > So instead attaching a patch that uses one socket per thread. > The patch also correct the stream throughput throttling calculation that was > 8000 times lower than what it should be. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
