[
https://issues.apache.org/jira/browse/CASSANDRA-8457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15527775#comment-15527775
]
Jason Brown edited comment on CASSANDRA-8457 at 9/27/16 11:31 PM:
------------------------------------------------------------------
TL;DR - I've addressed everything except for the interaction between
{{ClientHandshakeHandler}} and {{InternodeMessagingConnection}} (both are now
renamed). I've noticed the odd rub there, as well, for a while, and I'll take
some time to reconsider it.
re: "talking points"
- Backward compatibility - bit the bullet, and just yanked the old code
- streaming - [~slebresne] and I talked offline, and CASSANDRA-12229 will
address the streaming parts, and will be worked on/reviewed concurrently. Both
tickets will be committed together to avoid breaking streaming.
re: comments section 1
- Netty openssl - when I implemented this back in February, there was no
mechanism to use {{KeyFactoryManager}} with the OpenSSL implementation.
Fortunately, this has changed since I last checked in, so I've deleted the
extra {{keyfile}} and friends entries from the yaml/{{Config}}.
- "old code" - deleted now
- package javadoc - I absolutely want this :), I just want things to be more
solid code-wise before diving into that work.
- naming - names are now more consistent using In/Out (or Inbound/Outbound),
and use of client/server is removed.
re: comments section 2
- {{getSocketThreads()}} - I've removed this for now, and will be resolved with
CASSANDRA-12229
- {{MessagingService}} renames - done
- {{MessagingService#createConnection()}} In the previous implementation,
{{OutboundTcpConnectionPool}} only blocked on creating the threads for it's
wrapped {{OutboundTcpConnection}} instances (gossip, large, and small
messages). No sockets were actually opened until a message was actually sent to
that peer {{OutboundTcpConnection#connect()}}. Since we do not spawn a separate
thread for each connection type (even though we will have separate sockets), I
don't think it's necessary to block {{MessagingService#createConnection()}}, or
more correctly now, {{MessagingService.NettySender#getMessagingConnection()}}.
- "Seems {{NettySender.listen()}} always starts a non-secure connection" - You
are correct; however, looks like we've always been doing it that way (for
better or worse). I've gone ahead and made the change (it's a one liner, plus a
couple extra for error checking).
- {{ClientConnect#connectComplete}} - I've renamed the function to be more
accurate ({{connectCallback}}).
- {{CoalescingMessageOutHandler}} - done
Other issues resolved, as well. Branch has been pushed (with several commits at
the top) and tests running.
was (Author: jasobrown):
TL;DR - I've addressed everything except for the interaction between
{{ClientHandshakeHandler}} and {{Interno -deMessagingConnection}} (both are
noew renamed). I've noticed the odd rub there, as well, for a while, and I'll
take some time to reconsider it.
re: "talking points"
- Backward compatibility - bit the bullet, and just yanked the old code
- streaming - [~slebresne] and I talked offline, and CASSANDRA-12229 will
address the streaming parts, and will be worked on/reviewed concurrently. Both
tickets will be committed together to avoid breaking streaming.
re: comments section 1
- Netty openssl - when I implemented this back in February, there was no
mechanism to use {{KeyFactoryManager}} with the OpenSSL implementaion.
Fortunately, this has changed since I last checked in, so I've deleted the
extra {{keyfile}} and friends entries from the yaml/{{Config}}.
- "old code" - deleted now
- package javadoc - I absolutely want this :), I just want things to be more
solid code-wise before diving into that work.
- naming - names are now more consistent using In/Out (or Inbound/Outbound),
and use of client/server is removed.
re: comments section 2
- {{getSocketThreads()}} - I've removed this for now, and will be resolved with
CASSANDRA-12229
- {{MessagingService}} renames - done
- {{MessagingService#createConnection()}} In the previous implementation,
{{OutboundTcpConnectionPool}} only blocked on creating the threads for it's
wrapped {{OutboundTcpConnection}} instances (gossip, large, and small
messages). No sockets were actually opened until a message was actually sent to
that peer {{OutboundTcpConnection#connect()}}. Since we do not spawn a separate
thread for each connection type (even though we will have separate sockets), I
don't think it's necessary to block {{MessagingService#createConnection()}}, or
more correctly now, {{MessagingService.NettySender#getMessagingConnection()}}.
- "Seems {{NettySender.listen()}} always starts a non-secure connection" - You
are correct; however, looks like we've always been doing it that way (for
better or worse). I've gone ahead and made the change (it's a one liner, plus a
couple extra for error checking).
- {{ClientConnect#connectComplete}} - I've renmaed the function to be more
accurate ({{connectCallback}}).
- {{CoalescingMessageOutHandler}} - done
Other issues resolved, as well. Branch has been pushed (with several commits at
the top) and tests running.
> 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.4#6332)
