You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Sylvain Lebresne (Created) (JIRA)" <ji...@apache.org> on 2011/11/02 14:21:32 UTC
[jira] [Created] (CASSANDRA-3444) Secondary Index doesn't clean up
indexed CFS on remove and Streaming test failure.
Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
----------------------------------------------------------------------------------
Key: CASSANDRA-3444
URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
Project: Cassandra
Issue Type: Bug
Components: Core
Affects Versions: 1.0.0
Reporter: Sylvain Lebresne
Assignee: Sylvain Lebresne
Priority: Trivial
Fix For: 1.0.2
Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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-3444) Secondary Index doesn't clean
up indexed CFS on remove and Streaming test failure.
Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13145637#comment-13145637 ]
Jonathan Ellis commented on CASSANDRA-3444:
-------------------------------------------
Where I'm going is that if we've invalidated an index manager, destroying the record of what columns it indexed feels like the wrong kind of cleanup. Most often this is going to be called when the parent CF is dropped, so the index is never actually deleted from the schema (which is what clearing indexesByColumn implies) just excised as part of the drop.
TLDR: invalidate != removeIndexedColumn.
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.3
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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-3444) Secondary Index doesn't clean
up indexed CFS on remove and Streaming test failure.
Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144886#comment-13144886 ]
Jonathan Ellis commented on CASSANDRA-3444:
-------------------------------------------
I think this is showing that SIM needs some additional refactoring -- currently removeIndex is poorly named, because it leaves the index metadata around (which this patch tries to address) but invalidates the CFS representing it. This is addressed by 3437 v2 as well. (May have been addressed by v1, but I'm sure it is by v2. :)
So I would prefer to close this in favor of 3437.
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.3
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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-3444) Secondary Index doesn't clean
up indexed CFS on remove and Streaming test failure.
Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144267#comment-13144267 ]
Sylvain Lebresne commented on CASSANDRA-3444:
---------------------------------------------
So this patch kind of tries to solve the same problem than CASSANDRA-3437, at least as far as fixing trunk unit test failure is concerned. I kind of think that in a way the solution here is simpler/better (but maybe that just because it's my own). At the very I think that the added cleanup to SecondaryIndexManager.removeAllIndex() is the right thing to do.
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.3
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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] [Issue Comment Edited] (CASSANDRA-3444) Secondary Index
doesn't clean up indexed CFS on remove and Streaming test failure.
Posted by "Sylvain Lebresne (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144267#comment-13144267 ]
Sylvain Lebresne edited comment on CASSANDRA-3444 at 11/4/11 7:42 PM:
----------------------------------------------------------------------
So this patch kind of tries to solve the same problem than CASSANDRA-3437, at least as far as fixing trunk unit test failure is concerned. I actually think that both this ticket and CASSANDRA-3437 are legit in that the added cleanup to SecondaryIndexManager.removeAllIndex() is right and the cleaning of invalidate/unregisterSSTable of CASSANDRA-3437 is also reasonable. The two ticket have a different fix for the StreamingTest but which one is used doesn't matter.
was (Author: slebresne):
So this patch kind of tries to solve the same problem than CASSANDRA-3437, at least as far as fixing trunk unit test failure is concerned. I kind of think that in a way the solution here is simpler/better (but maybe that just because it's my own). At the very I think that the added cleanup to SecondaryIndexManager.removeAllIndex() is the right thing to do.
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.3
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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-3444) Secondary Index doesn't clean up
indexed CFS on remove and Streaming test failure.
Posted by "Sylvain Lebresne (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sylvain Lebresne updated CASSANDRA-3444:
----------------------------------------
Attachment: 0001-clean-indexes-post-remove-and-streaming-tests.patch
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.2
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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-3444) Secondary Index doesn't clean
up indexed CFS on remove and Streaming test failure.
Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13145342#comment-13145342 ]
Sylvain Lebresne commented on CASSANDRA-3444:
---------------------------------------------
I don't see 3437 v2 really addressing that in the sense that it seems to rename the method to invalidate, but since invalidate leaves the index unusable, I still can't see any reason for not clearing the indexes map in SIM a the end of invalidate. But it probably have no real consequence so far, I just think it would avoid future problems. And the main goal of this was to fix the test in trunk, which 3437 fixes too anyway, so I'm fine closing this.
> Secondary Index doesn't clean up indexed CFS on remove and Streaming test failure.
> ----------------------------------------------------------------------------------
>
> Key: CASSANDRA-3444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3444
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 1.0.0
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Trivial
> Fix For: 1.0.3
>
> Attachments: 0001-clean-indexes-post-remove-and-streaming-tests.patch
>
>
> The initial reason for that issue is because StreamingTransferTest is broken in trunk. It has been broken by CASSANDRA-3116 because the latter is too efficient. More precisely StreamingTransferTest create a CF with an index, then it calls unreferenceSSTables() on that CF to remove all sstable, and then try a transfer (that recreate the file and index basically). But when unreferenceSSTables() is called, it does fully remove the indexes in that the CFS object for the indexes stays. Post CASSANDRA-3116, this is problem because that CFS has been invalidated and thus nothing can be added back to it.
> Long story short, I believe that the fact that SecondaryIndexManager.removeAllIndex doesn't really unreference the CFS objects is not expected in the first place. The patch fixes that and update the StreamingTransferTest accordingly (fixing it as far as trunk is concerned).
--
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