[
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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
