[jira] [Comment Edited] (CASSANDRA-9462) ViewTest.sstableInBounds is failing

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

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

Benedict edited comment on CASSANDRA-9462 at 6/10/15 11:05 PM:
---------------------------------------------------------------

bq. However while looking into it I noticed it *also* likely has a bug (*which I have not updated the test to cover*) wherein a wrapped range will only yield the portion at the end of the token range, not the beginning. It looks like we may have call sites using this function that do not realise this, so it could be a serious bug, especially for repair.

This is actually the most serious problem that caused me to consider this a really important bug to fix, and I discovered it while trying to fix the more minor bug that ViewTest.sstableInBounds located. It looks like its semantics don't work at least as I would expect for Range objects, since it ignores any wrap-around portion. Whether or not this constitutes an actual bug in use is, not clear to me, but I could not convince myself it wasn't.

Fixing this other bug turned out to be surprisingly challenging and error prone, though, largely down to the inconsistencies of the current class hierarchy, and the confusion around semantics as Ariel points out. As you said on IRC, "it's a shit show, for sure" - I'm probably the second-most-eminent expert on these classes now, and given how much I mess up with them, it's evident I don't really understand them very well at all. So it was _my *view*_ that for the benefit of a majority of the project, we should take this opportunity to fix the hierarchy as well as fixing these issues, since _really_ their semantics should be tremendously simple to understand.


was (Author: benedict):
bq. However while looking into it I noticed it *also* likely has a bug (*which I have not updated the test to cover*) wherein a wrapped range will only yield the portion at the end of the token range, not the beginning. It looks like we may have call sites using this function that do not realise this, so it could be a serious bug, especially for repair.

This is actually the most serious problem that caused me to consider this a really important bug to fix, and I discovered it while trying to fix the more minor bug that ViewTest.sstableInBounds located. It looks like its semantics don't work at least as I would expect for Range objects, since it ignores any wrap-around portion.

Fixing this other bug turned out to be surprisingly challenging and error prone, though, largely down to the inconsistencies of the current class hierarchy, and the confusion around semantics as Ariel points out. As you said on IRC, "it's a shit show, for sure" - I'm probably the second-most-eminent expert on these classes now, and given how much I mess up with them, it's evident I don't really understand them very well at all. So it was _my *view*_ that for the benefit of a majority of the project, we should take this opportunity to fix the hierarchy as well as fixing these issues, since _really_ their semantics should be tremendously simple to understand.

> ViewTest.sstableInBounds is failing
> -----------------------------------
>
>                 Key: CASSANDRA-9462
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-9462
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Benedict
>            Assignee: Ariel Weisberg
>             Fix For: 3.x, 2.1.x, 2.2.x
>
>
> CASSANDRA-8568 introduced new tests to cover what was DataTracker functionality in 2.1, and is now covered by the lifecycle package. This particular test indicates this method does not fulfil the expected contract, namely that more sstables are returned than should be.
> However while looking into it I noticed it also likely has a bug (which I have not updated the test to cover) wherein a wrapped range will only yield the portion at the end of the token range, not the beginning. It looks like we may have call sites using this function that do not realise this, so it could be a serious bug, especially for repair.



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