[ 
https://issues.apache.org/jira/browse/CASSANDRA-12179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15401141#comment-15401141
 ] 

Robert Stupp commented on CASSANDRA-12179:
------------------------------------------

Some more comments:

* The {{if}} condition 
[here|https://github.com/apache/cassandra/commit/251c8d7499453452604fad1f577450fc979217e3#diff-b76a607445d53f18a98c9df14323c7ddR4245]
 is always wrong.
* The logic in {{StorageService.updateSnitch}} would effectively remove the 
{{DynamicEndpointSnitch}} mbean since it first instantiates a new dynamic 
snitch instance and then unregisters the mbean from the old one (not sure if 
that wouldn't cause an exception anyway).
* In c.yaml it is possible to pass in the "abbreviated"/short snitch class 
name, but {{updateSnitch}} always expects the full class name. (Not introduced 
by this patch)
* The methods {{changeDynamicResetIntervalInMs}} + {{changeBadnessThreshold}} + 
{{changeDynamicUpdateIntervalInMs}} can be merged into a single method.
* The logic to restart the tasks in {{DynamicEndpointSnicht}} is fine.

I took the freedom to update the patch in [this 
commit|https://github.com/apache/cassandra/commit/feb38d92d5e4bf0176d68c409478c7a21e1f9bf7]
 to address the above points and also:
* update the javadoc on the {{updateSnitch}} method
* reuse the {{DatabaseDescriptor.createEndpointSnitch}} method (and change it a 
bit) to allow short snitch class names
* merge the change* methods into a single {{applyConfigChanges}} method
* merge {{close}} and {{unregisterMBean}} methods
* fix the {{updateSnitch}} unregister-bbean logic and also refactor it to just 
cover the two cases when the snitch is changed and when it is not changed.

WDYT?

(Triggered new CI runs)

> Make DynamicEndpointSnitch dynamic_snitch_update_interval_in_ms a JMX Prop 
> ---------------------------------------------------------------------------
>
>                 Key: CASSANDRA-12179
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12179
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: sankalp kohli
>            Assignee: sankalp kohli
>            Priority: Trivial
>             Fix For: 3.0.x
>
>         Attachments: CASSANDRA-12179-3.0_v2.txt, CASSANDRA-12179_3.0.txt, 
> CASSANDRA-12179_3.0_v3.txt
>
>
> Need to expose dynamic_snitch_update_interval_in_ms so that it does not 
> require a bounce. This is useful for large clusters where we can change this 
> value and see the impact. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to