[ 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