cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-5286) Streaming 2.0
Date Wed, 19 Jun 2013 17:54:21 GMT

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

Sylvain Lebresne commented on CASSANDRA-5286:
---------------------------------------------

Remarks on that last version:
* In RangeStreamer, we use to throw a RuntimeException if some ranges hadn't been fetched
correctly. If my reading is correct, with this patch, we throw an AssertionError (with no
particular message) which is less helpful. Maybe the better option would be to have RangeStreamer.fetch()
return the StreamResultFuture and let the caller rethrow a more meaningful exception on error.
* In StorageService, when restoring replica count, we used to notify the endpoint even if
the streaming failed. Is there a reason this is not preserved by the patch?
* In StreamResultFuture, I think relying on a simple number of session might be fragile. Currently,
there is no protection against handleSessionComplete being called twice for the same session.
While I haven't checked carefully if this can happen, I can easily imagine that in the case
of a failure this function would be called twice in a racing way (maybe a node dies and we
get a socket error but also a signal from the failure detector at the same time). So I think
it could be worth protecting against this. Maybe StreamSession could get a UUID (that could
be useful for debugging anyway), and StreamResultFuture can keep a set of uncomplete sessions
instead of a single AtomicInteger?
* StreamManager can "leak" sessions if the exception thrown is not a StreamException. We should
either log a warning in that case, or change register to be
{noformat}
public void register(final StreamResultFuture result)
{
    result.addListener(new Runnable()
    {
        public run()
        {
            currentStreams.remote(result.planId);
        }
    }, MoreExecutors.sameThreadExecutor());
    currentStreams.put(result.planId, result);
}
{noformat}
* Feels weird to create a StreamPlan object on thre receiving end, since the Plan is mostly
a builder to initiate streaming. Besides, on the receiving side we'll only ever have one StreamSession
for the plan I believe. So I'd move the bulk of StreamPlan.execute to StreamResultFuture and
maybe just add a static StreamSession.startReceivingStream() method that does the equivalent
of StreamPlan.bind+execute.
* StreamPlan: can't we make planId and description final? In particular, I don't think we
have to wait for execute() to generate the planId. As for description, I was liking it a lot
more when this was an enum (if only because that enum documents in one place the different
use of streaming).
* In Outgoing/IncomingMessageHandler.run(), shouldn't we inform the session that something
is wrong on a SocketException?
* Seems like ConnectionHandler drops stream message on the floor without any kind of logging
if we're not connected. Would feel safer to either log a warning or just throw an exception
if that's not supposed to happen anyway.  Similarly, in IncomingMessageHandler, I don't think
StreamMessage.deserialize can return a null value, so we should have an assertion rather than
just silently ignoring what would be a programming error..
* I don't think StreamExecutor has to be a ListeningExecutor. So maybe we can have a proper
streaming stage (so the executor metrics are exposed alongside other stages).
* Nit: in ConnectionHandler.MessageHandler, terminated could just be a volatile.
* Nit: in OutgoingMessageHandler.run(), the 'else if (terminated())' is not useful since the
while loop already checks it.
* Nit: StreamManagerMBean.getCurrentStreamPlans() is redudant since the planId is part of
the StreamState.

I otherwise agree that JMX notification and streaming different sstable version could (and
really should) be left to followup ticket. I'm also fine pushing finer grained progress report
on reads to later.
                
> Streaming 2.0
> -------------
>
>                 Key: CASSANDRA-5286
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-5286
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Yuki Morishita
>              Labels: streaming
>             Fix For: 2.0 beta 1
>
>
> 2.0 is the good time to redesign streaming API including protocol to make streaming more
performant and reliable.
> Design goals that come up in my mind:
> *Better performance*
>   - Protocol optimization
>   - Stream multiple files in parallel (CASSANDRA-4663)
>   - Persistent connection (CASSANDRA-4660)
> *Better control*
>   - Cleaner API for error handling
>   - Integrate both IN/OUT streams into one session, so the components(bootstrap, move,
bulkload, repair...) that use streaming can manage them easily.
> *Better reporting*
>   - Better logging/tracing
>   - More metrics
>   - Progress reporting API for external client

--
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

Mime
View raw message