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