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