You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jonathan Ellis (JIRA)" <ji...@apache.org> on 2009/04/09 03:43:12 UTC
[jira] Created: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Review uses of FileStruct to make sure they are using decorated or raw keys correctly
-------------------------------------------------------------------------------------
Key: CASSANDRA-67
URL: https://issues.apache.org/jira/browse/CASSANDRA-67
Project: Cassandra
Issue Type: Bug
Reporter: Jonathan Ellis
Jun Rao commented in #58,
The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
ColumnFamilyStore.doFileCompaction()
In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697655#action_12697655 ]
Jun Rao commented on CASSANDRA-67:
----------------------------------
comments on v5:
1. remove the following unreferenced pacakges in FileStruct
java.util.Iterator
sun.reflect.generics.reflectiveObjects.NotImplementedException
2. occasional triggers the following failure in test. Not always reproducible. Wonder if it's another timing issue here.
[testng] FAILED: testGetCompactionBuckets
[testng] java.lang.AssertionError
[testng] at org.apache.cassandra.db.ColumnFamilyStoreTest.testGetCompactionBuckets(ColumnFamilyStoreTest.java:289)
[testng] ... Removed 22 stack frames
[testng]
Other than the above, the patch looks fine.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67-v5.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-67:
------------------------------------
Attachment: 67-v2.patch
Undecorate FS.key when calling isKeyInRanges per Jun's findings.
Note that the rest of anticompaction (and compaction) assume they are dealing with decorated keys, so that (and FS.key) are left alone.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis resolved CASSANDRA-67.
-------------------------------------
Resolution: Fixed
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67-v5.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697607#action_12697607 ]
Jun Rao commented on CASSANDRA-67:
----------------------------------
FileStructIterator seems unnecessary. It seems it's better to fold what's in FileStructIterator to FileStruct itself.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-67:
------------------------------------
Attachment: 67-v4.patch
Yes. v4 attached.
Also, to avoid confusion with the next() method from Iterator, renamed getNextKey() back to advance().
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697569#action_12697569 ]
Jun Rao commented on CASSANDRA-67:
----------------------------------
Reviewed this patch. Here are the comments.
1. FileStruct.getNextKey() should throw IOException (instead of RuntimeException) and let callers deal with it.
2. FileStruct.SeekTo() is not used.
3. FileStruct.iterator() gives user the impression that one can open up multiple independent iterators, but it is not.
4. In the new SSTable format, the block indexes are stored at the end of the file. If you encounter a blockindex key, you can be sure that you will never see a real key afterward.
So, need to change what FileStruct.getNextKey() does when incurring blockindex key.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jun Rao (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697591#action_12697591 ]
Jun Rao commented on CASSANDRA-67:
----------------------------------
3. Wouldn't it be better if FileStruct implements iterator interface directly, if there should be only 1 iterator expected on FileStruct?
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-67:
------------------------------------
Attachment: 67.patch
Clean up FileStruct and make it iterable. This improves the API and will also be necessary for range queries.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-67:
------------------------------------
Attachment: 67-v5.patch
version w/o iteration code, per IRC comments.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67-v5.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697658#action_12697658 ]
Jonathan Ellis commented on CASSANDRA-67:
-----------------------------------------
buckets test exception is unrelated. there is a separate ticket open for that.
committed w/ unused imports removed.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67-v5.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12697608#action_12697608 ]
Jonathan Ellis commented on CASSANDRA-67:
-----------------------------------------
Disagree. It's cleaner to not have it in the main namespace.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated CASSANDRA-67:
------------------------------------
Attachment: 67-v3.patch
1. Agreed.
2. It's going to be used by the range patch, but okay, to be consistent I will take it out of this one. :)
3. Added comment warning that iterators are not independent.
4. Good catch! I was just going off the old advance() method and didn't notice that. So when I get to the blockindex key I will treat it as if it were EOF.
v3 patch attached.
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (CASSANDRA-67) Review uses of FileStruct to make
sure they are using decorated or raw keys correctly
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-67?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis reassigned CASSANDRA-67:
---------------------------------------
Assignee: Jonathan Ellis
> Review uses of FileStruct to make sure they are using decorated or raw keys correctly
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-67
> URL: https://issues.apache.org/jira/browse/CASSANDRA-67
> Project: Cassandra
> Issue Type: Bug
> Reporter: Jonathan Ellis
> Assignee: Jonathan Ellis
> Attachments: 67-v2.patch, 67-v3.patch, 67-v4.patch, 67-v5.patch, 67.patch
>
>
> Jun Rao commented in #58,
> The problem is that FileStruct.key_ is referenced directly in 4 places. At least 2 of those places assume key_ to be the real key, instead of decorated key. These 2 places are in
> ColumnFamilyStore.doFileAntiCompaction() (key_ is assigned to lastkey, which is used in isKeyInRanges)
> ColumnFamilyStore.doFileCompaction()
> In the above places, key_ has to be undeocrated first. Also, we need to make key_ private in FileStruct and use getKey() for referencing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.