[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14697504#comment-14697504 ] Aleksey Yeschenko commented on CASSANDRA-9462: -- Committed as [3aa7308e8f86969158c8d919c3f77658ae7c4fc3|https://github.com/apache/cassandra/commit/3aa7308e8f86969158c8d919c3f77658ae7c4fc3] and merged with cassandra-3.0 and trunk, thanks. 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: 2.2.1, 3.0 beta 1 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14695203#comment-14695203 ] Aleksey Yeschenko commented on CASSANDRA-9462: -- 2.2 patch LGTM, and I agree w/ pushing the rest to CASSANDRA-9711. Couldn't get 3.0 version to apply, though. Need that, and proof of cassci happiness, to commit. 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.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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14695805#comment-14695805 ] Ariel Weisberg commented on CASSANDRA-9462: --- I rebased. That merge did not go quietly into the night. I think it conflicted with https://github.com/apache/cassandra/commit/ad8cad7c4d05fd5dea68fb274c81a102533ebe36#diff-b397cc5438b1a4be20f026c9ec9ecd6e Some of the resolution was straightforward. I think I got StreamSession right, and viewFilter from ColumnFamily store went away and became select() in View so I moved the comments there. Tests need to run again. 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.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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[
https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14611785#comment-14611785
]
Sylvain Lebresne commented on CASSANDRA-9462:
-
I'm honestly not entirely sure I understand from all the comments above what
problems we have identified exactly and what exactly the branch to review aims
to fix, so I'm gonna lay out what I understand and what I would suggest we do
and you can comment on what I've missed, what I misunderstood and where you
disagree.
The first problem we have, the one that I think this ticket is about, is that
{{ViewTest.sstableInBounds}} is failing. My understanding of that failure is
that {{View.sstableInBounds}} pretends (through its signature) returning
sstables for any type of {{AbstractBounds}} (so with any permutation of
inclusive/exclusive start and end) but it transform a bound into an
{{Interval}} which is always inclusive so it always acts as if
{{AbstractBounds}} was actually a {{Bound}}.
Now, is that an actual problem for the code? I don't think it is for 2 reasons:
# the use of the interval tree is an optimization in the first place and having
a sstable returned that has no actual data for our range isn't a problem. And
given how incredibly unlikely it is in practice that a sstable is returned
unnecessarily due an exclusive bound being treated as inclusive, it's not even
a concrete performance issue.
# it's more anecdotal but at least some of the bounds passed to
{{sstableInBounds}} comes from a call to (the now misnamed)
{{Range.makeRowRange}}, which uses {{Token.maxKeyBound()}} which returns a
{{PartitionPosition}} that cannot be any real {{DecoratedKey}} (it's by design
bigger than any key having the original token). It follows that it doesn't
matter what type of {{AbstractBounds}} is returned by {{Range.makeRowRange}},
it will always select the same keys. Therefore passing its result to
{{sstableInBounds}} ends up being fine.
A second problem, which if I understand correctly is the most serious one
[~benedict] is referring to, is that there is a number of places where we pass
an {{AbstractBounds}} and the code silently assumes it is not wrapped (please
correct me if I misunderstood that). And it would indeed do the wrong thing if
we mistakenly passed a wrapping range. It's hard to say for sure that we never
make that mistake since it's hard to exhaustively list all the places where we
make those silent assertions, but at first glance I think we're good because:
* For reads, we unwrap stuff pretty early (in
{{StorageProxy.getRestrictedRanges}}).
* For streaming, we also clearly unwraps stuff in
{{StreamSession.addTransferRanges}}.
So sum up, my initial take away is that:
# The test failure does not expose a true bug in the code (at least not one
with user-visible consequence).
# The code make silent assertions regarding {{AbstractBounds}} in a number of
places making it easy to mess up, even though to the best of my knowledge there
is no evidence that we do mess up.
# The underlying cause of this is that the {{AbstractBounds}} API is confusing,
messy and makes mistake easy.
So, as I've said before, I'm all for rethinking the {{AbstractBounds}} API, but
that's not a minor undertaking and a different ticket. I've actually opened
CASSANDRA-9711 since I don't think we had one?.
Now in the short term, what I would suggest for this ticket is to add concrete
assertions in the place we've identified where there is silent assertions. I've
pushed a suggestion for this
[here|https://github.com/pcmanus/cassandra/commits/9462]. It's obviously a
band-aid, but it's simple enough that I think we can commit this in 2.1+ (it's
mostly stating assertions more clearly). That patch will force us to modify the
failing test so it only test inclusive bounds, thus fixing it, but my patch is
against 2.1 so the test changes are not included).
Now, there seems to be a third problem pointed by [~aweisberg] regarding some
of the behaviors of {{AbstractBounds}} methods: {{isWrapAround}} and
{{isEmpty}} more specifically.
Regarding {{Range.isWrapAround}}, it's definitively true that the existing
semantic is a weird and inconsistent regarding the min value. That said, and
unless we can identify actual bugs due to this semantic, I would prefer leaving
to CASSANDRA-9711 the task of fixing it since changing it is a lot of risk for
little and short-lived gains if we agree that we should do CASSANDRA-9711
anyway.
For {{isEmpty}}, to have {{isEmpty(bound(1, false), bound(1, true)) == true}}
actually feels right to me (alternatively, we could make isEmpty bitch because
that range is nonsensical). The behaviors with {{MIN}} are more obviously
broken, though the method is only called by {{SSTableScanner}} that, I think,
never pass it {{MIN}}. So one option could be to simply assert that in the
method (again, knowing
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[
https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14611854#comment-14611854
]
Sylvain Lebresne commented on CASSANDRA-9462:
-
I'll note that the new assertions of my patch have actually found what I think
is a genuine bug in {{SizeEstimatesRecorder}} where it wasn't unwrapping the
range (making it count only the size for the upper part of a wrapped range).
I've pushed the simple fix on [the
branch|https://github.com/pcmanus/cassandra/commits/9462].
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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[
https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14611861#comment-14611861
]
Benedict commented on CASSANDRA-9462:
-
Just to confirm that it generally sounds like we're on the same page, i.e. that
you've correctly interpreted my statements, and that I'm on board with your
approach. The minutiae of {{isEmpty}} (and other) semantics I've not got a
particular interest in, so long as they are consistent and well documented.
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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14580564#comment-14580564 ] Sylvain Lebresne commented on CASSANDRA-9462: - bq. I haven't really look at the test failure though, so I don't understand yet what the problem is and why you think AbstractBounds is to be blame (if you could clarify, that would be awesome). I would still appreciate some clarification on this. 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14580749#comment-14580749 ] Ariel Weisberg commented on CASSANDRA-9462: --- JIRA just ate my response which really really sucks. IntervalTree for (0, 0), (1,1) search for IncludingExcludingBound (0,1) returns a wrong answer. Benedict tried to fix it for SSTableIntervalTree by overriding search. I then tried to make all the tests pass with that implementation which lead to writing tests for the inconsistent (or maybe not, hard to tell) implementation of methods inside AbstractBounds and Range, making them consistent, and then fixing the fallout from that. I am going to try again and see if I can just fix IntervalTree. I didn't expect to spend a week getting the tests passing on the original fix, but it was probably necessary anyways just to understand what is going on. AFAIK existing code works around the inconsistencies and higher level modules have some tests that maybe validate that they are using AbstractBounds and family correctly. It's also possible the entire thing was a waste and we should just make IntervalTree reject IncludingExcludingBounds? This is kind of why I want someone with more context to review. 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[
https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14580901#comment-14580901
]
Ariel Weisberg commented on CASSANDRA-9462:
---
I took the tests from the branch and ran them on trunk just to see the
difference in what I implemented. On trunk in RangeTest these two assertions
fail. On the branch I operated under the assumption that min() on the right is
max() and thus it can't wrap.
{noformat}
assertFalse(makeRange(MIN, MIN).isWrapAround());
assertFalse(makeRange(3, MIN).isWrapAround());
{noformat}
There are also failures for isEmpty(). These all pass on the branch but fail on
trunk.
{noformat}
assertFalse(AbstractBounds.isEmpty(bound(1, false), bound(1, true)));
assertFalse(AbstractBounds.isEmpty(bound(1, true), bound(MIN, false)));
assertFalse(AbstractBounds.isEmpty(bound(1, false), bound(MIN, false)));
assertFalse(AbstractBounds.isEmpty(bound(1, false), bound(MIN, true)));
assertFalse(AbstractBounds.isEmpty(bound(1, true), bound(MIN, true)));
assertFalse(AbstractBounds.isEmpty(bound(MIN, true), bound(MIN, false)));
assertFalse(AbstractBounds.isEmpty(bound(MIN, false), bound(MIN, false)));
assertFalse(AbstractBounds.isEmpty(bound(MIN, false), bound(MIN, true)));
{noformat}
I would say that this disagreement is what prompts a lot of the changes.
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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14580894#comment-14580894 ] Ariel Weisberg commented on CASSANDRA-9462: --- I found out why Benedict had to implement search for SSTableIntervalTree. IntervalTree relies on comparable for the bounds and nothing else so it doesn't know if the bounds are inclusive/exclusive. Now whether SSTableIntervalTree needs to handle all the different kinds of bounds? Maybe that is a question still worth asking. If that is the case, then the changes are basically fall out from me fixing the original SSTableIntervalTree search and adding tests for the methods and then making them consistent. 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14581200#comment-14581200 ] Benedict commented on CASSANDRA-9462: - 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14579869#comment-14579869 ] Michael Shuler commented on CASSANDRA-9462: --- The dtest for this dev branch has run for long periods on two occasions and has needed to be killed. Builds #1 and #5. Just a little info :-) 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[
https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14574757#comment-14574757
]
Sylvain Lebresne commented on CASSANDRA-9462:
-
bq. I've inserted an assertion in bounds construction to ensure we never
create a bound that wraps that is not a Range.
This is already asserted in the actual implementations ctors (Bounds,
IncludingExcludingBounds and ExcludingBounds).
bq. any input on the history of this class
What I can tell you is that we used to only have the {{Range}} class, which is
the one that is the most used since our token ranges are really {{Range}}).
That's also why {{Range}} has some methods the other don't have like
{{normalize}} and {{deoverlap}} (we never bothered generalizing those methods
to the other implementation because we haven't had a need for it). That's also
why the other version are not allowed to wrap: we hadn't needed it so far so
that was simpler.
bq. if implementing unwrap() for all versions of AbstractBounds is illadvised
for some reason?
Well, as said above, the other versions can't wrap by construction so their
{{unwrap}} is properly implemented.
I haven't really look at the test failure though, so I don't understand yet
what the problem is and why you think {{AbstractBounds}} is to be blame (if you
could clarify, that would be awesome). I'm not particularly in love with the
current AbstractBound classes implementation so I have sympathy for the idea of
cleaning them up a bit, but I think we still need to understand what problem
we're trying to solve and which risks/benefits there is before rushing into it.
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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14575216#comment-14575216 ] Ariel Weisberg commented on CASSANDRA-9462: --- I have something for this. It's not done. At the very least I want to add tests for the additional and involved functionality in this ticket I a still getting a handle on what the correct behavior for these things is. I need to take a look at unwrap to see if it is supposed to work differently for different kinds of inclusive/exclusive combinations. https://github.com/apache/cassandra/compare/trunk...aweisberg:C-9462?expand=1 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14570376#comment-14570376 ] Benedict commented on CASSANDRA-9462: - I've pushed a branch [here|https://github.com/belliottsmith/cassandra/tree/9516] to attempt to deal with this, since I had a half-completed implementation from CASSANDRA-8568 (that when I realised the scale of the problem, I decided was out of scope for that ticket). In doing so, I noticed some more problems / inconsistencies: AbstractBounds provides an unwrap() method, but it is a no-op unless the bound is a Range. This isn't at all clear, and I suspect we have been using AB with the assumption they could also wrap. I've inserted an assertion in bounds construction to ensure we never create a bound that wraps that is not a Range. This branch, as a result, is exploratory: whether or not CI breaks, we should probably remove the assertions and implement the unwrap() method, as introducing assertions like those could cause currently broken functionality not caught in tests to fail hard in the field. Ultimately, I think I would prefer that we flatten the different AbstractBounds classes into one Bounds implementation for this ticket. Which probably means different patches for 2.1/2.2 and 3.0. 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 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)
[jira] [Commented] (CASSANDRA-9462) ViewTest.sstableInBounds is failing
[ https://issues.apache.org/jira/browse/CASSANDRA-9462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14570403#comment-14570403 ] Benedict commented on CASSANDRA-9462: - [~slebresne]: do you have any thoughts to this restructure, any input on the history of this class, and, particularly, on if implementing unwrap() for all versions of AbstractBounds is illadvised for some reason? 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 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)
