cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Brown (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-8457) nio MessagingService
Date Sat, 24 Jun 2017 22:03:00 GMT

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

Jason Brown commented on CASSANDRA-8457:
----------------------------------------

bq. Nit: there has been a few Netty minor released since the one in the branch, maybe worth
upgrading (since we change it anyway).
there were a few unstable netty releases so I skipped those. 4.1.12 is good, but I need a
fix in {{Lz4FrameEncoder}} which should be in 4.1.13.

bq. MessageOutHandler.AUTO_FLUSH_THRESHOLD feels like a magic constants. At least a comment
on why it's a reasonable value would be nice.
I've added a comment to the constant, but the value was the value that should have been for
the {{Lz4FrameEncoder}} (16k). The value from the [existing code|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/OutboundTcpConnection.java#L85]
is 64k, so I've reinstated that. Note that the previous {{OTC.BUFFER_SIZE}} can be seen also
as a flush threshold, as well, as it is a [parameter to the output stream|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/OutboundTcpConnection.java#L471].
Also discovered that {{OTC.BUFFER_SIZE}} is configurable, and made the new version configurable,
as well.

bq. OutboundHandshakeHandler.WRITE_IDLE_MS: what's the rational behind 10 seconds? Not saying
it feels wrong per-se, but wondering if there is more than a gut-feeling to it, and I'd kind
of suggest exposing a system property to change that default.
I believe 10 seconds was a value [~aweisberg] and I came up with, but I've made it configurable
now. 

bq. There is still a few TODO:JEB in the code: would be good to resolve/clear them if we're
going to a more final branch.
Done, most of them I had already done on the CASSANDRA-12229 branch, but now backported to
this one.

bq. Largely a nit, but its a tad confusing that OutboundConnectionParams contains a OutboundMessagingConnection.
It feels like saying "The parameters you need for an outbound connection is ... an outbound
connection". Maybe a simple renaming would make this clearer, though it feels maybe this could
be cleanup up a bit further.
Hmm, yeah. I need the {{OutboundMessagingConnection}} for {{ChannelWriter#handleMessageFuture()}}
so it can react to the outcome of attempting to send the message. wrt a cleanup, the least-worst
idea I have is to move the error handling functionality of {{ChannelWriter#handleMessageFuture()}}
to {{OutboundMessageConnection}}, as that's the only thing in {{ChannelWriter}} that requires
{{OutboundMessageConnection}}. In {{OutboundConnectionParams}} we can pass a reference to
an "error consumer" (not fully thought out but something like {{BiConsumer<QueuedMessage,
Throwable>}}, that's a function on {{OutboundMessageConncetion}}), and {{ChannelWriter#handleMessageFuture()}}
can invoke that instead of having a reference to a {{OutboundMessageConncetion}}. wdyt?

bq. The handling of "backlogged" messages and channel writability feels a bit complex. For
instance, it looks like MessageOutHandler.channelWritabilityChanged can potentially silently
drop a message every time it's called.
Good catch, fixed.

bq. The CI links listed with the branch a bunch of comments ago are completely out of dates.

updated the ticket with a pointer to the circleci utests.

bq. Some basic benchmark results wouldn't hurt either ...
Attaching to this ticket, as it's a non-trivial martrix to test all the combinations of compression,
coalescing, jdk-based TLS, and openssl-based TLS. 

bq. Maybe you could create a parent ticket of which this and CASSANDRA-12229 would be sub-tasks
where we could focus the testing/benchmarking/final discussions on the whole thing?
done as CASSANDRA-13628


> nio MessagingService
> --------------------
>
>                 Key: CASSANDRA-8457
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8457
>             Project: Cassandra
>          Issue Type: New Feature
>            Reporter: Jonathan Ellis
>            Assignee: Jason Brown
>            Priority: Minor
>              Labels: netty, performance
>             Fix For: 4.x
>
>
> Thread-per-peer (actually two each incoming and outbound) is a big contributor to context
switching, especially for larger clusters.  Let's look at switching to nio, possibly via Netty.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message