[jira] [Commented] (CASSANDRA-10572) SinglePartitionNamesCommand::canRemoveRow omits counter cells it shouldn't

["Sylvain Lebresne (JIRA)" <ji...@apache.org>]

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

Sylvain Lebresne commented on CASSANDRA-10572:
----------------------------------------------

I've pushed a patch [here|https://github.com/pcmanus/cassandra/commits/10572].

A very trivial fix for this would be to exclude counters like we exclude collections in {{SinglePartitionNamesCommand::canRemoveRow}}. However, the reality is that it's particularly judicious to use the timestamp-ordered path for either of those (collections or counters), since we'd be guaranteed to read all sstable anyway and so reading sstable sequentially (materializing everything in memory) is not the best strategy. So the best strategy is probably to have counters and collections use the non timestamp ordered path. Doing so however kind of obviate the reason for splitting {{SinglePartitionReadCommand}} in 2 subclass (names and slice comands), so the patch remove that. Basically, it pulls the 2 possible path (timestamp-orderer and all-sstables-simultaneously) in {{SinglePartitionReadCommand}} and pick the timestamp-ordered one only for names query without either counters or collections. As such, the patch is bigger that one would expect, but that's mostly because it moves some code around and update the code accordingly.

> SinglePartitionNamesCommand::canRemoveRow omits counter cells it shouldn't
> --------------------------------------------------------------------------
>
>                 Key: CASSANDRA-10572
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10572
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Aleksey Yeschenko
>            Assignee: Sylvain Lebresne
>             Fix For: 3.0.0
>
>
> Like with collections, we can never be sure we've exhausted all the sstables when reducing the filter. The sstables can contain legacy local shards that need to be summed up. Unless we look into the content of the cells (?), or until we get rid of the pre-2.1 values, we must include all the sstables in the read (so long as clusterings overlap).
> This was not an issue with 2.1/2.2 as we've forced {{collectAllData}} always on that path, but now we don't, and {{SinglePartitionNamesCommand}} should handle the case.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)