You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Andres de la Peña (Jira)" <ji...@apache.org> on 2021/04/15 11:37:00 UTC
[jira] [Comment Edited] (CASSANDRA-16601) Flaky CassandraIndexTest
[ https://issues.apache.org/jira/browse/CASSANDRA-16601?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17322106#comment-17322106 ]
Andres de la Peña edited comment on CASSANDRA-16601 at 4/15/21, 11:36 AM:
--------------------------------------------------------------------------
I have managed to reproduce the failure running the entire test class in our internal multiplexer, with 22 failures in 200 runs ([here|https://jenkins-dse.build.dsinternal.org/view/Parameterized/job/parameterized-testall/789/]). Indeed the problem is that the other tests also write entries in the {{system.IndexInfo}} table and, since {{CQLTester.afterTest}} cleanup is asynchronous, that table can change in any moment. CASSANDRA-15565 solved the problem of having indexes from previous tests in the table by taking note of how many indexes were in the table at the beginning of the test. The missed part was that {{CQLTester.afterTest}} can remove them in the middle of the test.
The proposed PR indeed avoids that risk by just checking the specific entry for the tested index, but I think it defeats part of the purpose of the test, which aims to verify that creating, rebuilding and dropping an index doesn't create unexpected entries in {{system.IndexInfo}}. If we wish to not do this check, we should probably get rid of the calls to {{assertRowsIgnoringOrderAndExtra}}, the picking of the initial query size (which is always zero), and the comments of the form {{"check that there are no other rows in the built indexes table"}}.
Alternatively, if we want to keep checking that no unexpected entries are added to {{system.IndexInfo}}, we should make sure that no other test writes in that table while we do our checks. The simplest way I can think of is making the test a dtest, since those use a dedicated cluster. I gave it a go [here|https://github.com/adelapena/cassandra/commit/1efcc5781bdfebc39692da6ea49bfbde7e49f866].
Also, now that we are probably going to get rid of parallel test execution, we could add a method in {{CQLTester}} to wait for the cleanup tasks of previous tests, which should guarantee us an empty {{system.IndexInfo}}, as it's done [here|https://github.com/adelapena/cassandra/commit/8cf7145b8437f616e8085169cc3829197961031c]. That method could also be helpful for other similar tests, and will allow us to restore the simpler pre-15565 shape of the test.
wdyt?
was (Author: adelapena):
I have managed to reproduce the failure running the entire test class in our internal multiplexer, with 22 failures in 200 runs ([here|https://jenkins-dse.build.dsinternal.org/view/Parameterized/job/parameterized-testall/789/]). Indeed the problem is that the other tests also write entries in the {{system.IndexInfo}} table and, since {{CQLTester.afterTest}} cleanup is asynchronous, that table can change in any moment. CASSANDRA-15565 solved the problem of having indexes from previous tests in the table by taking note of how many indexes were in the table at the beginning of the test. The missed part was that {{CQLTester.afterTest}} can remove them in the middle of the test.
The proposed PR indeed avoids that risk by just checking the specific entry for the tested index, but I think it defeats part of the purpose of the test, which aims to verify that creating, rebuilding and dropping an index doesn't create unexpected entries in {{system.IndexInfo}}. If we wish to not do this check, we should probably get rid of the calls to {{assertRowsIgnoringOrderAndExtra}}, the picking of the initial query size (which is always zero), and the comments of the form {{"check that there are no other rows in the built indexes table"}}.
Alternatively, if we want to keep checking that no unexpected entries are added to {{system.IndexInfo}}, we should make sure that no other test writes in that table while we do our checks. The simplest way I can think of is making the test a dtest, since those use a dedicated cluster. I gave it a go [here|https://github.com/adelapena/cassandra/commit/1efcc5781bdfebc39692da6ea49bfbde7e49f866].
Also, now that we are probably going to get rid of parallel test execution, we could add a method in {{CQLTester}} to wait for the cleanup tasks of previous tests, which should guarantee us an empty {{system.IndexInfo}}, as it's done [here|https://github.com/adelapena/cassandra/commit/8cf7145b8437f616e8085169cc3829197961031c].
wdyt?
> Flaky CassandraIndexTest
> ------------------------
>
> Key: CASSANDRA-16601
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16601
> Project: Cassandra
> Issue Type: Bug
> Components: Test/unit
> Reporter: Berenguer Blasi
> Assignee: Berenguer Blasi
> Priority: Normal
> Fix For: 4.0-rc
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> See failure [here|https://ci-cassandra.apache.org/job/Cassandra-trunk/436/testReport/junit/org.apache.cassandra.index.internal/CassandraIndexTest/indexCorrectlyMarkedAsBuildAndRemoved_cdc/]
> {noformat}
> Error Message
> expected:<1> but was:<0>
> Stacktrace
> junit.framework.AssertionFailedError: expected:<1> but was:<0>
> at org.apache.cassandra.index.internal.CassandraIndexTest.indexCorrectlyMarkedAsBuildAndRemoved(CassandraIndexTest.java:588)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org