[jira] [Commented] (CASSANDRA-10059) Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
[
https://issues.apache.org/jira/browse/CASSANDRA-10059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15038000#comment-15038000
]
Branimir Lambov commented on CASSANDRA-10059:
-
Sorry, {{// filtered}} referred to column filtering. Changed to say that
explicitly and moved parameters generation out of the way to make the structure
clearer.
Rebased and updated, also reflecting your other comments:
|[code|https://github.com/blambov/cassandra/tree/10059]|[utests|http://cassci.datastax.com/view/Dev/view/blambov/job/blambov-10059-testall/]|[dtests|http://cassci.datastax.com/view/Dev/view/blambov/job/blambov-10059-dtest/]|
> Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
>
>
> Key: CASSANDRA-10059
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10059
> Project: Cassandra
> Issue Type: Test
> Components: Local Write-Read Paths
>Reporter: Benedict
>Assignee: Branimir Lambov
> Fix For: 3.0.1, 3.1
>
>
> Follow up to CASSANDRA-9932. The test coverage for AbstractBTreePartition and
> its hierarchy is entirely indirect. That is not to say it is not covered, but
> we may have some unexplored behaviour. Coverage for BTree is also missing
> around a couple of edges, and the gaps should be filled in.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10059) Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
[
https://issues.apache.org/jira/browse/CASSANDRA-10059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15035782#comment-15035782
]
Sylvain Lebresne commented on CASSANDRA-10059:
--
Slightly late to the review, sorry. Test is convincing in general and fixes
looks good but a few small remarks on the test:
* In the {{test(Supplier ...)}} method, the lack of both braces and indentation
makes this hard to read.
* In the main {{testIter()}} method, there is a sub-part about {{// filtered}},
but while this filter on the expected input, it doesn't filter the actual
iterator. The test still pass because the row generated by the test never have
(cell) tombstones, so there is nothing filtered, but it's still kind of wrong.
We can randomly generate some tombstone when generating rows and use
{{UnfilteredRowIterators.filter()}} on the iterator from the partition, but
that's more a test of {{UnfilteredRowIterators}} that of the partition
implementation itself, so I'm fine if you prefer just removing that case here.
* Nitpicking here: one assumption of a partition is that it should not contain
data that it deletes itself, which in practice mean that any cell covered by a
marker should have a timestamp greater than marker deletion time. The test
mostly respect that thanks to the use of the current time for row timestamp but
a random number <= {{KEY_RANGE}} for range markers, but that feels a bit like a
luck as is. Maybe using {{KEY_RANGE + 1}} as timestamp for the row cells with
a short comment would be more explicit.
Could you also make sure the branch is rebased and CI are ran on it.
> Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
>
>
> Key: CASSANDRA-10059
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10059
> Project: Cassandra
> Issue Type: Test
> Components: Local Write-Read Paths
>Reporter: Benedict
>Assignee: Branimir Lambov
> Fix For: 3.0.1, 3.1
>
>
> Follow up to CASSANDRA-9932. The test coverage for AbstractBTreePartition and
> its hierarchy is entirely indirect. That is not to say it is not covered, but
> we may have some unexplored behaviour. Coverage for BTree is also missing
> around a couple of edges, and the gaps should be filled in.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10059) Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
[
https://issues.apache.org/jira/browse/CASSANDRA-10059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15004056#comment-15004056
]
Branimir Lambov commented on CASSANDRA-10059:
-
Updated [branch|https://github.com/blambov/cassandra/tree/10059] with fixes for
iteration of range tombstones in {{RangeTombstoneList}}. Forward iteration was
broken on boundaries, reporting invalid {{\[x, x)}} ranges if a range boundary
matches a slice boundary, and reverse iteration was completely broken. The
former shouldn't have caused any real problems, but the latter could cause
reverse range queries to report deleted data which gets fixed with repair or
compaction.
Patch also relaxes the requirement for {{SearchIterator}} to report null on
already queried names as enforcing it would add too much complexity for
questionable benefit.
> Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
>
>
> Key: CASSANDRA-10059
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10059
> Project: Cassandra
> Issue Type: Test
>Reporter: Benedict
>Assignee: Branimir Lambov
> Fix For: 3.1
>
>
> Follow up to CASSANDRA-9932. The test coverage for AbstractBTreePartition and
> its hierarchy is entirely indirect. That is not to say it is not covered, but
> we may have some unexplored behaviour. Coverage for BTree is also missing
> around a couple of edges, and the gaps should be filled in.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-10059) Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy
[ https://issues.apache.org/jira/browse/CASSANDRA-10059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15002178#comment-15002178 ] Branimir Lambov commented on CASSANDRA-10059: - Patch is not ready for review, I am now working on addressing the identified issues. > Test Coverage and related bug-fixes for AbstractBTreePartition and hierarchy > > > Key: CASSANDRA-10059 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10059 > Project: Cassandra > Issue Type: Test >Reporter: Benedict >Assignee: Branimir Lambov > Fix For: 3.1 > > > Follow up to CASSANDRA-9932. The test coverage for AbstractBTreePartition and > its hierarchy is entirely indirect. That is not to say it is not covered, but > we may have some unexplored behaviour. Coverage for BTree is also missing > around a couple of edges, and the gaps should be filled in. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
