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-8457) nio MessagingService
Date Mon, 29 May 2017 09:12:05 GMT

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

Sylvain Lebresne commented on CASSANDRA-8457:
---------------------------------------------

Had a quick look at the last changes. A few remaining questions/remarks:
* 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.
* MessageOutHandler.AUTO_FLUSH_THRESHOLD feels like a magic constants. At least a comment
on why it's a reasonable value would be nice.
* 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.
* {{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.
* 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, and I'll admit not being sure if this can have unintened
consequence in overload situations. In general, it would be really great to have some testing
of this patch under a fair amount of load (and overload) to make sure things work as intended.
* Nit: there has been a few Netty minor released since the one in the branch, maybe worth
upgrading (since we change it anyway).

Other than that, and related to one comment above, it would be nice to see some tests run
for this (including upgrade tests as this is particularly sensible to any MessagingService
changes). The CI links listed with the branch a bunch of comments ago are completely out of
dates. Some basic benchmark results would hurt either since that completely change a fairly
sensible part of the code. Of course, it's not that meaningful without CASSANDRA-12229, which
makes this a bit awkward. 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?


> 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.3.15#6346)

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


Mime
View raw message