[
https://issues.apache.org/jira/browse/CASSANDRA-16444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17293772#comment-17293772
]
Alexandre Dutra edited comment on CASSANDRA-16444 at 3/3/21, 9:54 AM:
----------------------------------------------------------------------
[~blerer] I investigated the change introduced by CASSANDRA-15867.
In fact, this change simply fixed a test that was failing for a while.
Doing a git bisect, I found that the commit that introduced the test regression
is in fact c4064dd: [Allow recovery from the cases when CQL-created compact
sense tables have bytes in EmptyType
columns|https://github.com/apache/cassandra/commit/c4064dd80e427aec7c04e8e2e1e4630d6c8087b6#diff-0e5a142c13247885605175ace136c549391fb6c778877f91f589fd94131c241b].
This change introduced an assertion inside {{AbstractType.writeValue()}}:
{code:java}
assert valueLengthIfFixed < 0 || value.remaining() ==
valueLengthIfFixed : String.format("Expected exactly %d bytes, but was %d",
valueLengthIfFixed, value.remaining());
{code}
Without the assert, the incorrect 8-bytes value created by
{{testInsertingIncorrectValuesIntoAgeIndex}} gets written; with the assertion,
it doesn't, and the {{AssertionError}} bubbles up, messing up the store state.
The author of CASSANDRA-15867 simply added a try-catch block around the test to
avoid the test failure. But I wonder if we shouldn't simply disable this test,
since it violates an invariant expressed by the new assertion.
Oddly enough, this assertion was not ported to trunk, which basically conserved
the pre-c4064dd behavior. This is why this test
({{testInsertingIncorrectValuesIntoAgeIndex}}) is not dangerous on trunk: it
passes normally and doesn't leave the store in an inconsistent state; therefore
it doesn't need a try-catch block nor any ugly band-aid to fix the store state.
was (Author: adutra):
[~blerer] I investigated the change introduced by CASSANDRA-15867.
In fact, this change simply fixed a test that was failing for a while.
Doing a git bisect, I found that the commit that introduced the test regression
is in fact c4064dd: [Allow recovery from the cases when CQL-created compact
sense tables have bytes in EmptyType
columns|https://github.com/apache/cassandra/commit/c4064dd80e427aec7c04e8e2e1e4630d6c8087b6#diff-0e5a142c13247885605175ace136c549391fb6c778877f91f589fd94131c241b].
This change introduced an assertion inside {{AbstractType.writeValue()}}:
{code:java}
assert valueLengthIfFixed < 0 || value.remaining() ==
valueLengthIfFixed : String.format("Expected exactly %d bytes, but was %d",
valueLengthIfFixed, value.remaining());
{code}
Without the assert, the incorrect 8-bytes value created by
{{testIndexMemtableSwitching}} gets written; with the assertion, it doesn't,
and the {{AssertionError}} bubbles up, messing up the store state. The author
of CASSANDRA-15867 simply added a try-catch block around the test to avoid the
test failure. But I wonder if we shouldn't simply disable this test, since it
violates an invariant expressed by the new assertion.
Oddly enough, this assertion was not ported to trunk, which basically conserved
the pre-c4064dd behavior. This is why this test is not dangerous on trunk: it
passes normally and doesn't leave the store in an inconsistent state; therefore
it doesn't need a try-catch block nor any ugly band-aid to fix the store state.
> Fix flaky test testMultiExpressionQueriesWhereRowSplitBetweenSSTables -
> org.apache.cassandra.index.sasi.SASIIndexTest
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-16444
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16444
> Project: Cassandra
> Issue Type: Bug
> Components: Test/unit
> Reporter: David Capwell
> Assignee: Alexandre Dutra
> Priority: Normal
> Fix For: 4.0-beta
>
>
> https://app.circleci.com/pipelines/github/dcapwell/cassandra/862/workflows/d2b10373-5bd1-4895-a738-1c28587cae62/jobs/5136
> {code}
> junit.framework.AssertionFailedError: []
> at
> org.apache.cassandra.index.sasi.SASIIndexTest.testMultiExpressionQueriesWhereRowSplitBetweenSSTables(SASIIndexTest.java:589)
> at
> org.apache.cassandra.index.sasi.SASIIndexTest.testMultiExpressionQueriesWhereRowSplitBetweenSSTables(SASIIndexTest.java:468)
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
