You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jeff Hodges (JIRA)" <ji...@apache.org> on 2009/08/10 11:31:14 UTC
[jira] Created: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
SystemTable.initMetadata throws an NPE when called twice
--------------------------------------------------------
Key: CASSANDRA-358
URL: https://issues.apache.org/jira/browse/CASSANDRA-358
Project: Cassandra
Issue Type: Improvement
Reporter: Jeff Hodges
While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-2.patch
358-1.patch mistakenly incremented the returned generation niumber
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-1.patch, 358-2.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-3.patch
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-1.patch, 358-2.patch, 358-3.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742513#action_12742513 ]
Jonathan Ellis commented on CASSANDRA-358:
------------------------------------------
> it doesn't cater for an updateToken call after which it will start returning stale data
the right fix there is to update the cached SMD in updateToken, then.
it seems pretty clear to me that SMD was intended to be a singleton in the original design.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v8.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742400#action_12742400 ]
Jonathan Ellis commented on CASSANDRA-358:
------------------------------------------
Why do we need the equals and hashcode methods on StorageMetadata?
I don't like defining those if they're not used, because by defining them, you imply that they _are_. :)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-358:
-------------------------------------
Attachment: 358-v11.patch
The thing is, we _want_ all copies of SMD to be updated when we change the token or Bad Things Could Happen. The only way to do that sanely is to make SMD a singleton.
Here is v11, which you can think of as either v10 with SMD a singleton, or v8 with the call to setStorageId moved into updateToken and synchronized. :)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v10.patch, 358-v11.patch, 358-v8.patch, 358-v9.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: (was: 358-6.patch)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12741958#action_12741958 ]
Bill de hOra commented on CASSANDRA-358:
----------------------------------------
{quote}the explanation is simple but how does taking care of state three double the existing code size? {quote}
That would be my preference for organizing clumps of code using Compose Method, in this case to make what the main does function explicit. No big deal. I'll submit a functional patch anon.
{quote}no underscore_ in variables is preferred. if a class is mixed cleanup should convert to no-underscores.{quote}
np, I thought that was the preferred idiom.
{quote}
avoid splitting methods up that are not reusable or perform a simple expression. if a block of code is used in only one place, prefer leaving it inline (use whitespace to denote logical blocks, and comments if necessary). obviously there are gray areas but methods like "notFound" and "applyAddColumn" are definitely over the line.
{quote}
Sure; in this code base there is code whose design intent isn't clear, initMetadata being an example ;)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-4.patch, 358-5.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: (was: 358-4.patch)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-7.patch
use consts
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-6.patch
functional patch with less refactoring; unit and nostests pass against r803211
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-4.patch, 358-5.patch, 358-6.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-4.patch
rolled up patch into single commit; extended the tests
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-3.patch, 358-4.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (CASSANDRA-358)
SystemTable.initMetadata throws an NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12741674#action_12741674 ]
Bill de hOra edited comment on CASSANDRA-358 at 8/11/09 4:00 AM:
-----------------------------------------------------------------
This thing really didn't want to be called twice in a running jvm.
Changed to add 3 states
* called and no LocationInfo CF - create it (as per current code)
* called and LocationInfo CF found but no - bump generation it (as per current code)
* called and LocationInfo CF found but we know it was just created - return the available LocationInfo CF data as is
The NPE happens I think because the query filter doesn't include the token in its columns (generation included and found but not token). Even then you still need to know whether it's a restart or a new create so the first minted token is stashed in a static, which is how it knows LocationInfo CF has been created in this jvm runtime.
was (Author: dehora):
This thing really didn't want to be called twice in a running jvm.
Changed to add 3 states
* called and no LocationInfo CF - create it (as per current code)
* called and LocationInfo CF found but no - bump generation it (as per current code)
* called and LocationInfo CF found but we know it was just created - return the available LocationInfo CF data as is
The NPE happens I think because the query filter doesn't include the token in its columns (generation included and found but not token). Even then you still need to know whether it's a restart or a new create so the first minted token is stashed in a static, which is how know it's just been created in this jvm runtime.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-3.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: (was: 358-3.patch)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-4.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742041#action_12742041 ]
Jonathan Ellis commented on CASSANDRA-358:
------------------------------------------
> Sure; in this code base there is code whose design intent isn't clear, initMetadata being an example ;)
Oh, I make no claims about how well the existing code base adheres to our prefered style or readability in general, only the direction we'd like to move it in. :)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742407#action_12742407 ]
Jonathan Ellis commented on CASSANDRA-358:
------------------------------------------
I think I understand: because you're returning a new SMD if the token is already created, but you want it to be "equal to" the other ones.
This looks like it can cause subtle bugs -- generation is supposed to be "global" to the node. I think caching the SMD rather than the token would be cleaner and less error-prone.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (CASSANDRA-358)
SystemTable.initMetadata throws an NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742534#action_12742534 ]
Bill de hOra edited comment on CASSANDRA-358 at 8/12/09 12:26 PM:
------------------------------------------------------------------
> it seems pretty clear to me that SMD was intended to be a singleton in the original design.
That might be the case, but I suspect it just was designed with the assumption it would only ever be iniitalised once, which doesn't hold anymore. I've been looking at how StorageMetadata is used in StorageService (it's only used there). It's kind of broken - StorageService.updateToken calls SystemTable.updateToken but then updates its StorageMetadata field directly - StorageService should ask SystemTable directly for the StorageMetadata, leaving it to manage cache/refs, and be refielded instead of "Update the storageMetadata cache" directly as per the comment in L492.
> the right fix there is to update the cached SMD in updateToken, then.
Ok, I think I'll go with the cache notion, but it needs some small work in StorageService as well.
was (Author: dehora):
> it seems pretty clear to me that SMD was intended to be a singleton in the original design.
That might be the case, but I suspect it just was designed with the assumption it would only ever be iniitalised once, which doesn't hold anymore. I've been looking at how StorageMetadata is used in StorageService (it's only used there). It's kind of broken - StorageService.upin L492dateToken calls SystemTable.updateToken but then updates its StorageMetadata field directly - StorageService should ask SystemTable directly for the StorageMetadata, leaving to manage cache/refs, and be refielded instead of "Update the storageMetadata cache" directly as per the comment in L492.
> the right fix there is to update the cached SMD in updateToken, then.
Ok, I think I'll go with the cache notion, but it needs some small work in StorageService as well.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v8.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: (was: 358-2.patch)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-3.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743696#action_12743696 ]
Hudson commented on CASSANDRA-358:
----------------------------------
Integrated in Cassandra #168 (See [http://hudson.zones.apache.org/hudson/job/Cassandra/168/])
make StorageTable.StorageMetadata a singleton, so multiple initMetadatas are safe.
patch by Bill de hOra and jbellis; reviewed by Michael Greene for
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Assignee: Bill de hOra
> Fix For: 0.4
>
> Attachments: 358-5.patch, 358-7.patch, 358-v10.patch, 358-v11.patch, 358-v8.patch, 358-v9.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Michael Greene (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12743442#action_12743442 ]
Michael Greene commented on CASSANDRA-358:
------------------------------------------
Looks good to me, passes all internal and public tests. +1
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Fix For: 0.4
>
> Attachments: 358-5.patch, 358-7.patch, 358-v10.patch, 358-v11.patch, 358-v8.patch, 358-v9.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742512#action_12742512 ]
Bill de hOra commented on CASSANDRA-358:
----------------------------------------
I had a similar idea to the v8 patch, but it doesn't cater for an updateToken call after which it will start returning stale data (subtle but definite design bug). I think I can merge the ideas though if you're happy to synchronize the class across updateToken as well.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v8.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12741925#action_12741925 ]
Jonathan Ellis commented on CASSANDRA-358:
------------------------------------------
thanks for the patch!
the explanation is simple but how does taking care of state three double the existing code size? skimming the patch it looks like we might be overengineering things a bit here. could you split this into "cleanup" and "add functionality" patches? it's much easier to follow that way. (then the review can skim the cleanup for sanity checking, and focus on reviewing the new functionality.)
a couple style notes:
no underscore_ in variables is preferred. if a class is mixed cleanup should convert to no-underscores.
avoid splitting methods up that are not reusable or perform a simple expression. if a block of code is used in only one place, prefer leaving it inline (use whitespace to denote logical blocks, and comments if necessary). obviously there are gray areas but methods like "notFound" and "applyAddColumn" are definitely over the line.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-4.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Michael Greene (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Greene updated CASSANDRA-358:
-------------------------------------
Component/s: Core
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: (was: 358-1.patch)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-3.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-358:
-------------------------------------
Attachment: 358-v8.patch
my proposed patch.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v8.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-1.patch
This thing really didn't want to be called twice in a running jvm.
Changed to add 3 states
* called and no LocationInfo CF - create it (as per current code)
* called and LocationInfo CF found but no - bump generation it (as per current code)
* called and LocationInfo CF found but we know it was just created - return the available LocationInfo CF data as is
The NPE happens I think because the query filter doesn't include the token in its columns (generation included and found but not token). Even then you still need to know whether it's a restart or a new create so the first minted token is stashed in a static, which is how know it's just been created in this jvm runtime.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-1.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-v10.patch
variation of 358-v8.patch that works when the token is updated and has coarse grained sync
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v10.patch, 358-v8.patch, 358-v9.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-358) SystemTable.initMetadata throws
an NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12742534#action_12742534 ]
Bill de hOra commented on CASSANDRA-358:
----------------------------------------
> it seems pretty clear to me that SMD was intended to be a singleton in the original design.
That might be the case, but I suspect it just was designed with the assumption it would only ever be iniitalised once, which doesn't hold anymore. I've been looking at how StorageMetadata is used in StorageService (it's only used there). It's kind of broken - StorageService.upin L492dateToken calls SystemTable.updateToken but then updates its StorageMetadata field directly - StorageService should ask SystemTable directly for the StorageMetadata, leaving to manage cache/refs, and be refielded instead of "Update the storageMetadata cache" directly as per the comment in L492.
> the right fix there is to update the cached SMD in updateToken, then.
Ok, I think I'll go with the cache notion, but it needs some small work in StorageService as well.
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v8.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-v9.patch
variation of 358-v8.patch that works when the token is updated and has fine grained sync
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Jeff Hodges
> Attachments: 358-5.patch, 358-7.patch, 358-v10.patch, 358-v8.patch, 358-v9.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-358) SystemTable.initMetadata throws an
NPE when called twice
Posted by "Bill de hOra (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-358?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bill de hOra updated CASSANDRA-358:
-----------------------------------
Attachment: 358-5.patch
removed "final" keywords (doesn't look like anything is being reassigned)
> SystemTable.initMetadata throws an NPE when called twice
> --------------------------------------------------------
>
> Key: CASSANDRA-358
> URL: https://issues.apache.org/jira/browse/CASSANDRA-358
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jeff Hodges
> Attachments: 358-4.patch, 358-5.patch
>
>
> While this is not the expected use case of it, SystemTable.initMetadata throws an NPE when called twice in the same process. The error points to tokenColumn being null even through cf is not at line 111.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.