You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "gabriele renzi (JIRA)" <ji...@apache.org> on 2009/12/12 13:28:18 UTC
[jira] Created: (CASSANDRA-631) possible NPE in StorageProxy?
possible NPE in StorageProxy?
-----------------------------
Key: CASSANDRA-631
URL: https://issues.apache.org/jira/browse/CASSANDRA-631
Project: Cassandra
Issue Type: Bug
Affects Versions: 0.5
Environment: all
Reporter: gabriele renzi
insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
{{{
logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
}}}
this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "gabriele renzi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
gabriele renzi updated CASSANDRA-631:
-------------------------------------
Attachment: CASSANDRA-631-big.patch
CASSANDRA-631-tiny.patch
changes the hunhinted ->hinted as per summary.
The -tiny patch only does that change .
The -big patch also removes other compile warnings from the file (generics, synthetic accessors) and avoids using useless allocations (Collections.max(Arrays.AsList(new ary[a,b])) seems unnecessary when there is only the need to compare two objects and commons-lang already provides a good enough method)
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Affects Versions: 0.5
> Environment: all
> Reporter: gabriele renzi
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "gabriele renzi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789733#action_12789733 ]
gabriele renzi commented on CASSANDRA-631:
------------------------------------------
forgot to say: with both patches I have a tet failure
[junit] Testcase: testImportSuperCf(org.apache.cassandra.tools.SSTableImportTest): Caused an ERROR
[junit] Invalid localDeleteTime read: -2099059532
[junit] java.io.IOException: Invalid localDeleteTime read: -2099059532
[junit] at org.apache.cassandra.db.SuperColumnSerializer.deserialize(SuperColumn.java:368)
[junit] at org.apache.cassandra.db.SuperColumnSerializer.deserialize(SuperColumn.java:1)
[junit] at org.apache.cassandra.db.filter.SSTableNamesIterator.<init>(SSTableNamesIterator.java:103)
[junit] at org.apache.cassandra.db.filter.NamesQueryFilter.getSSTableColumnIterator(NamesQueryFilter.java:69)
[junit] at org.apache.cassandra.tools.SSTableImportTest.testImportSuperCf(SSTableImportTest.java:63)
at r889834 I also see it without the patches though :)
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Affects Versions: 0.5
> Environment: all
> Reporter: gabriele renzi
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789748#action_12789748 ]
Jonathan Ellis commented on CASSANDRA-631:
------------------------------------------
applied -tiny to 0.5 and trunk, good catch.
-big removes private from a bunch of variables for no reason, so left that part out, but applied the generics fixes to trunk (although fixing generics only to add in an ObjectUtils call requiring a cast seems a little schizophrenic to me; saving a 2-element array allocation in get_range_slice is like picking up a teaspoon of sand from the seashore :)
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792160#action_12792160 ]
Hudson commented on CASSANDRA-631:
----------------------------------
Integrated in Cassandra #291 (See [http://hudson.zones.apache.org/hudson/job/Cassandra/291/])
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789783#action_12789783 ]
Jonathan Ellis commented on CASSANDRA-631:
------------------------------------------
Interesting, I didn't know about that.
But if it's actually in a performance sensitive place, wouldn't the JIT inlining will take care of it?
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis resolved CASSANDRA-631.
--------------------------------------
Resolution: Fixed
Fix Version/s: 0.9
0.5
Assignee: gabriele renzi
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "gabriele renzi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789777#action_12789777 ]
gabriele renzi commented on CASSANDRA-631:
------------------------------------------
Sorry, I wrote it in the patch comment but I failed to express myself clearly: in my setup I have basically all warnings turned on, and for access of private fields from inner classes there are some performance-related ones (the need for compiler-generated accessor methods) which are not present in case of package visibility.
It's a silly reason but I just use all possible warnings (+pmd, +findbugs) in my code, and this happen to be in the set, I'll just turn this off if it seems unnecessary. Shall I update the CodeStyle wiki page to point this out?
Same for generics, mostly a warning removal thingy.
Regarding ObjectUtils, we save an array, a list and an iterator object instantiation, so it's 3 spoons! (that should be probably optimized away with escape analysis anyway :)
Thanks for applying the patch.
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-631:
-------------------------------------
Component/s: Core
Affects Version/s: (was: 0.5)
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-631) possible NPE in StorageProxy?
Posted by "gabriele renzi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789863#action_12789863 ]
gabriele renzi commented on CASSANDRA-631:
------------------------------------------
you can check it with JAD if you disable the inner class analisys, the code
{{{
public class C {
private static int var = 10;
public static class Inner {
public int meth(){
return var;
}
}
}
}}}
gives back from the C.class
{{{
public class C
{
public C()
{
}
static int access$000()
{
return var;
}
private static int var = 10;
}
}}}
and for C$Inner.class
{{{
public class C$Inner
{
public C$Inner()
{
}
public int meth()
{
return C.access$000();
}
}
}}}
I _believe_ this is because inner classes had to be fitted over an existing class format in java 1.1, but I'm not sure.
I agree that these method are most probably going to be optimized away, as I said I just changed the code because they are in the default warnings set and I can simply change my warnings setup (and eventually update the CodeStyle wiki so the next person who comes across them will know it).
> possible NPE in StorageProxy?
> -----------------------------
>
> Key: CASSANDRA-631
> URL: https://issues.apache.org/jira/browse/CASSANDRA-631
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Environment: all
> Reporter: gabriele renzi
> Assignee: gabriele renzi
> Fix For: 0.5, 0.9
>
> Attachments: CASSANDRA-631-big.patch, CASSANDRA-631-tiny.patch
>
> Original Estimate: 0.17h
> Remaining Estimate: 0.17h
>
> insert() in StorageProxy contains a logging statement that refers to a possibly un-initialized variable
> {{{
> logger.debug("insert writing key " + rm.key() + " to " + unhintedMessage.getMessageId() + "@" + hintedTarget + " for " + target);
> }}}
> this could happen if getHintedEndpointMap(rm.key(), naturalEndpoints) returns only elements for which target.equals(hintedTarget) returns false, which seems possible to me.
> Looking at the code I get the feeling the reference should probably be to 'hintedMessage', instead of "unhintedMessage", if not so an
> assert statement could be appropriate
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.