cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-3872) Sub-columns removal is broken in 1.1
Date Mon, 13 Feb 2012 11:16:59 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-3872?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13206806#comment-13206806
] 

Sylvain Lebresne commented on CASSANDRA-3872:
---------------------------------------------

I do not pretend this reduce line of codes, but I do think that it makes it easier to not
make subtle mistakes.

Currently, there is a mismatch between how Column (the class) and the two IColumnContainer
classes (CF and SC) handles getLocalDeletionTime() for non-deleted. The former uses MAX_VALUE,
the latter uses MIN_VALUE. The lack of consistency alone is annoying but as long as SC lives
it is made much worst by the fact that SC is both a IColumn and a IColumnContainer.

The attached patch tries to make things more consistent. The localDeletionTime is here for
the purpose of tombstone garbage collection, so it seems to me that it is cleaner to use it
for that purpose and that purpose only. In other words, with this patch, {{(getLocalDeletionTime()
< gcbefore)}} tells you without ambiguity if you're dealing with a gcable tombstone or
not.

Now there is the fact that live but empty containers are not returned to the user. I believe
that was one of the reason of using MIN_VALUE for live containers. But imho this is a hack
and it's much more clear in removeDeleted to read:
{noformat}
if (cf.getColumnCount() == 0 && (!cf.isMarkedForDelete() || cf.getLocalDeletionTime()
< gcBefore))
{noformat}
which directly translate into: if the cf is empty and it's either a gcable tombstone or a
live cf, we can skip it, rather that having to check the code of ColumnFamily to understand
why that does skip live empty CF *and* to have to remember each time you use CF.localDeletionTime
that it may be MIN_VALUE for non-deleted CF and assert if it matters or not.
                
> Sub-columns removal is broken in 1.1
> ------------------------------------
>
>                 Key: CASSANDRA-3872
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-3872
>             Project: Cassandra
>          Issue Type: Bug
>    Affects Versions: 1.1.0
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>             Fix For: 1.1.0
>
>         Attachments: 3872.patch
>
>
> CASSANDRA-3716 actually broke sub-columns deletion. The reason is that in QueryFilter.isRelevant,
we've switched in checking getLocalDeletionTime() only (without looking for isMarkedForDelete).
But for columns containers (in this case SuperColumn), the default local deletion time when
not deleted is Integer.MIN_VALUE. In other words, a SC with only non-gcable tombstones will
be considered as not relevant (while it should).
> This is caught by two unit tests (RemoveSuperColumnTest and RemoveSubColumnTest) that
are failing currently.

--
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

        

Mime
View raw message