[jira] [Comment Edited] (CASSANDRA-8457) nio MessagingService

["Jason Brown (JIRA)" <ji...@apache.org>]

    [ 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)