[
https://issues.apache.org/jira/browse/CASSANDRA-8086?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14303713#comment-14303713
]
Joshua McKenzie commented on CASSANDRA-8086:
--------------------------------------------
h4. General
* We might want to consider not registering this into the ChannelPipeline in
Server.java unless configured in the .yaml as that would guarantee no
performance degradation for the default use case. Would mean we couldn't
modify it on the fly.
* Should be able to map InetAddress to Long instead of String to Long, save on
toString calls and string comparison and also make the code a touch cleaner.
* Not immediately clear why we need
StorageProxy.setNativeTransportMaxConcurrentConnections(...) - are we looking
to modify this on the fly?
* DatabaseDescriptor.setNativeTransportMaxConcurrentConnectionsPerIp(...) added
but not used - can remove
* Some kind of unit test would be nice, though I'm not sure how much of a
precedent there is in the code-base for testing our ChannelHandlers in
isolation.
h4. Performance considerations:
* The added synchronization around a HashMap on every new and closed connection
may be an issue. We could instead use a ConcurrentHashMap with a non-blocking
approach which should be less code though arguably a bit more complex to reason
about. Whether or not we want to pursue that route largely depends on whether
our connections are long-running or ephemeral which I'm not sure of - [~tjake]
/ [~benedict] - could either of you speak to that?
h4. Formatting nits:
(ConnectionLimitHandler.java)
* Don't need line break on declaration of connectionsPerClient, redundant
specification of types in <> on HashMap decl as well
* Space after if on line 53
* removing {} around the guts of the perIpCount checks and connectionsPerClient
placement makes it a little more compact and retains readability
** same for channelClosed when manipulating connectionsPerClient
In general it's looking good. W/formatting and String -> InetAddress
conversion, we may be good to commit based on the above depending on the
performance aspect.
> Cassandra should have ability to limit the number of native connections
> -----------------------------------------------------------------------
>
> Key: CASSANDRA-8086
> URL: https://issues.apache.org/jira/browse/CASSANDRA-8086
> Project: Cassandra
> Issue Type: Bug
> Reporter: Vishy Kasar
> Assignee: Norman Maurer
> Fix For: 2.1.3, 2.0.13
>
> Attachments:
> 0001-CASSANDRA-8086-Allow-to-limit-the-number-of-native-c-2.0.patch,
> 0001-CASSANDRA-8086-Allow-to-limit-the-number-of-native-c.patch,
> 0001-CASSANDRA-8086-Allow-to-limit-the-number-of-native-c.txt
>
>
> We have a production cluster with 72 instances spread across 2 DCs. We have a
> large number ( ~ 40,000 ) of clients hitting this cluster. Client normally
> connects to 4 cassandra instances. Some event (we think it is a schema change
> on server side) triggered the client to establish connections to all
> cassandra instances of local DC. This brought the server to its knees. The
> client connections failed and client attempted re-connections.
> Cassandra should protect itself from such attack from client. Do we have any
> knobs to control the number of max connections? If not, we need to add that
> knob.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
