You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jun Rao (JIRA)" <ji...@apache.org> on 2009/05/28 03:25:45 UTC

[jira] Created: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

Refactor different getRow calls in Table.java to more specific name and add more logging information
----------------------------------------------------------------------------------------------------

                 Key: CASSANDRA-202
                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
             Project: Cassandra
          Issue Type: Improvement
            Reporter: Jun Rao
            Assignee: Jun Rao
         Attachments: issue202.patchv1

The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis commented on CASSANDRA-202:
------------------------------------------

> By the same token, we should make all thrift APIs getRow calls

We _should_ have get_column, get_key_range, and a bunch of get_slice overloads.  The reason we do not is that thrift does not support overloads, not because it's actually better to have get_slice_by_names, get_slice_from, etc.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis commented on CASSANDRA-202:
------------------------------------------

I'm skeptical that this would actually be useful in a real-world situation.  If you can't open a sstable because of a bug in cassandra then it's almost certain the actual culprit was in the flush or compact code, so knowing the extra parameters at the time of getRow helps not at all.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis updated CASSANDRA-202:
-------------------------------------

    Attachment: 202.patch

Here is a patch implementing my proposed re-throw.  If we do end up needed more query-specific details to troubleshoot a specific failure let's add it then.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: 202.patch, issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jun Rao updated CASSANDRA-202:
------------------------------

    Attachment: issue202.patchv2

Attach v2 of the patch. It basically rethrows the IOException with a more detailed message.

For debugging purpose, it's useful to know all parameters of the call that causes the exception.


> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715254#action_12715254 ] 

Jun Rao commented on CASSANDRA-202:
-----------------------------------

Suppose that an SSTable is corrupted. Then, it would be useful to use the exact same api to figure out how the file gets corrupted, before fixing it.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis reopened CASSANDRA-202:
--------------------------------------


oops, we never did reach a resolution on the logging issue.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715133#action_12715133 ] 

Jun Rao commented on CASSANDRA-202:
-----------------------------------

I am not sure about this argument. By the same token, we should make all thrift APIs getRow calls. Does that make it clear to the users? Probably not.

The only commonality among those methods is that they access a single row. The fact that there is a key parameter already indicates that.

The problem with catching only at CFS level is that one can't get parameters other than key and cf. Other parameters are useful for debugging purpose.


> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Michael Greene updated CASSANDRA-202:
-------------------------------------

    Component/s: Core

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>             Fix For: 0.4
>
>         Attachments: 202.patch, issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis resolved CASSANDRA-202.
--------------------------------------

    Resolution: Fixed

committed 202.patch

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: 202.patch, issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Hudson commented on CASSANDRA-202:
----------------------------------

Integrated in Cassandra #112 (See [http://hudson.zones.apache.org/hudson/job/Cassandra/112/])
    rethrow IOException with more details.  patch by jbellis; reviewed by Jun Rao for 


> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: 202.patch, issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis commented on CASSANDRA-202:
------------------------------------------

Adding key/cf info to the IOExceptions is a good thing, but it should be added to the IOException message instead of logging a separate debug before the error, and it should be done where the actual IOE comes from, not in the ex-getRow calls (which is overly broad, we should only add key/cf info to IOEs that actually involve the key, e.g. SF.next, not SSTable.init or bufIn.readUTF() -- or we muddy the water instead of clarifying it.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis commented on CASSANDRA-202:
------------------------------------------

the more i think about it the more i think that changing the method names instead of using overloads of getRow makes it less clear rather than more -- using the same method name emphasizes the similarity of the functionality which is appropriate here.

as for the re-throw, if you put the try/catch/rethrow around

                ColumnFamily columnFamily = fetchColumnFamily(key, cf, filter, file);

in CFS, then you would only have to add it in one place.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jonathan Ellis commented on CASSANDRA-202:
------------------------------------------

> The problem with catching only at CFS level is that one can't get parameters other than key and cf. Other parameters are useful for debugging purpose. 

The other parameters aren't going to be involved in an IOException so the argument for 5x code duplication for the sake of including them  is weak.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12720947#action_12720947 ] 

Jun Rao commented on CASSANDRA-202:
-----------------------------------

This is fine. I realize that there is enough debug info in weakReadLocal and weakReadRemote in StorageProxy that provides the relevant context when an exception happens.

I still think that it's better to rename the 5 versions of getRow in Table though.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: 202.patch, issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CASSANDRA-202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12715146#action_12715146 ] 

Jun Rao commented on CASSANDRA-202:
-----------------------------------

Now that we have to use different names in thrift, wouldn't using those same names internally make things clearer for development purpose? Otherwise, a developer is forced to remember the name mapping.

On the surface, arguments other than key and CF won't trigger IOExceptions. However, they can trigger IOExceptions indirectly. For example, for the get_slice_by_names call, depending on the list of columns provided, we may need to open different set of SSTables.  Therefore, for debugging purpose, it is useful to know the exact parameters that triggered the exceptions.


> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1, issue202.patchv2
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (CASSANDRA-202) Refactor different getRow calls in Table.java to more specific name and add more logging information

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

Jun Rao updated CASSANDRA-202:
------------------------------

    Attachment: issue202.patchv1

Attache a patch. Most of the changes are in Table.java. Others are just refactoring.

> Refactor different getRow calls in Table.java to more specific name and add more logging information
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-202
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Jun Rao
>            Assignee: Jun Rao
>         Attachments: issue202.patchv1
>
>
> The Table class has five different getRow methods. We need to change them to more appropriate names. Also, when an IOException occurs, it would be useful to log the parameters of the request. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.