You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Tyler Hobbs (Created) (JIRA)" <ji...@apache.org> on 2011/10/20 00:45:11 UTC

[jira] [Created] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

StorageService.setMode() is used inconsistently
-----------------------------------------------

                 Key: CASSANDRA-3388
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
             Project: Cassandra
          Issue Type: Bug
          Components: Core
    Affects Versions: 1.0.0
            Reporter: Tyler Hobbs
            Priority: Trivial


{{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.

To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131099#comment-13131099 ] 

Jonathan Ellis commented on CASSANDRA-3388:
-------------------------------------------

We're talking about this, right?

{code}
    private void setMode(String m, boolean log)
{code}

Why would we even expose this over JMX?  It's not intended to be public.
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131357#comment-13131357 ] 

Jonathan Ellis commented on CASSANDRA-3388:
-------------------------------------------

Ah, okay.

setMode isn't exposed though, which is as it should be since it's informational only.  Setting it wouldn't actually change any internal state to match.
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Tyler Hobbs (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135215#comment-13135215 ] 

Tyler Hobbs commented on CASSANDRA-3388:
----------------------------------------

A couple of comments on v1 of the patch:
* I think you accidentally lower-cased 'Leaving' on one line
* Everything is done consistently now, but I'm a little bit concerned about this being accidentally broken in the future.  Would you mind either splitting the mode (currently just the string prefix) into a separate parameter or at least documenting the setMode() method with a note about how it should be used?

Thanks, Sylvain.
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>             Fix For: 1.0.2
>
>         Attachments: 3388.patch
>
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Tyler Hobbs (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131361#comment-13131361 ] 

Tyler Hobbs commented on CASSANDRA-3388:
----------------------------------------

Right, I wasn't suggesting that setMode() be exposed externally at all.  I was merely saying that the way it's used _internally_ is inconsistent.  (And since the attribute is readable through JMX, being consistent matters.)
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Sylvain Lebresne (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-3388:
----------------------------------------

    Attachment: 3388_v2.patch

Yeah right. I was going with the 'smallest diff' approach but I agree we can make that cleaner. v2 does so by adding a Mode enumeration. A few details (that I think are ok but are worth mentioning):
* the patch changes the semantic of the getOperationMode JMX attribute to only return the "mode", not the message part. It can be changed back but I'm not sure the message is really useful outside of the log itself and it feels slightly cleaner that way.
* the modes are in caps. Again, feels more simple, natural but that can change.
* I've changed the last "mode" from Draining to Drained. Feels more useful/right that way.
* 
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>             Fix For: 1.0.2
>
>         Attachments: 3388.patch, 3388_v2.patch
>
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131739#comment-13131739 ] 

Jonathan Ellis commented on CASSANDRA-3388:
-------------------------------------------

Ah, I get it now.  I was confused by the description of "making this attribute more usable through JMX."
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Tyler Hobbs (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131351#comment-13131351 ] 

Tyler Hobbs commented on CASSANDRA-3388:
----------------------------------------

It's named getOperationMode().
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Sylvain Lebresne (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-3388:
----------------------------------------

    Attachment: 3388.patch
    
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>             Fix For: 1.0.2
>
>         Attachments: 3388.patch
>
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Sylvain Lebresne (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sylvain Lebresne updated CASSANDRA-3388:
----------------------------------------

    Fix Version/s: 1.0.2
    
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>             Fix For: 1.0.2
>
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131257#comment-13131257 ] 

Jonathan Ellis commented on CASSANDRA-3388:
-------------------------------------------

I don't see setMode or getMode in StorageServiceMBean.  I'm lost, how is it exposed through JMX?
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Tyler Hobbs (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13131153#comment-13131153 ] 

Tyler Hobbs commented on CASSANDRA-3388:
----------------------------------------

Yes, that's the method.  Sorry, I forgot that there was also a "log" parameter.

It's a fairly reliable way to figure out if a node is joining, moving, decommissioning, or has finished decommissioning.  I'm open to alternative ways of discovering this reliably.  I assumed that's why this was exposed through JMX in the first place.
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CASSANDRA-3388) StorageService.setMode() is used inconsistently

Posted by "Tyler Hobbs (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-3388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135330#comment-13135330 ] 

Tyler Hobbs commented on CASSANDRA-3388:
----------------------------------------

Minor note: {{setMode(Mode.DECOMMISSIONED, null, true);}} could omit the {{null}} parameter.

Other than that, it looks perfect to me, and I agree with your thoughts.  Thanks!

+1
                
> StorageService.setMode() is used inconsistently
> -----------------------------------------------
>
>                 Key: CASSANDRA-3388
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3388
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.0.0
>            Reporter: Tyler Hobbs
>            Priority: Trivial
>             Fix For: 1.0.2
>
>         Attachments: 3388.patch, 3388_v2.patch
>
>
> {{StorageService.setMode()}}, which ends up setting the OperationMode attribute of the related mbean, is used inconsistently.  In most places, it's used like "{{setMode("MODE: details")}}, but in a few places, it's used more like a normal log message.
> To make this attribute more usable through JMX, {{setMode()}} should have a signature like {{setMode(mode, details)}}, where the mode parameter could be an enum (or even just a string, the main thing is just being consistent).  The OperationMode JMX attribute should definitely remain a string, though.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira