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

Benjamin Lerer edited comment on CASSANDRA-16444 at 3/2/21, 4:05 PM:
---------------------------------------------------------------------

Thanks for the patches and the deep analysis.

* I believe that even if it does not appear as a problem right now we should 
make the tests more robusts. 2 things that strike me are that the tables are 
not cleaned up between the tests and that {{forcedFlushes}} are used but the 
tests do not disable the automatic flushes.

I would change the {{cleanupData}} method into:
{code}
    private static void cleanupData()
    {
        stores().forEach(ColumnFamilyStore::truncateBlocking);
    }

    private static Stream<ColumnFamilyStore> stores()
    {
        Keyspace ks = Keyspace.open(KS_NAME);
        return ks.getMetadata().tables.stream().map(t -> 
ks.getColumnFamilyStore(t.name));
    }
{code}

and the {{cleanUp}} method into:
{code}
    @Before
    public void cleanUp()
    {
        cleanupData();
    }
{code} 
That would ensure that we clean all the tables between the different tests.

I would also add 
{{stores().forEach(ColumnFamilyStore::disableAutoCompaction);}} at the end of 
the {{loadSchema()}} method to ensure that there are no race condition with 
automatic compactions.

* Regarding the timestamp fix, would it not make sense to use some thing like:
{code}
    private static long timestamp = 0;

    private static long nextTimestamp()
    {
        timestamp += 1000;
        return timestamp;
    }
{code}
rather than specifying the timestamp for each call to {{loadData}}?

* Regarding {{testInsertingIncorrectValuesIntoAgeIndex}} problem in 3.11, it 
seems that the test has been changed as part of CASSANDRA-15867 and I am not 
convinced that this change was the correct one. If you roll back that change, 
do you still see the same issue?


was (Author: blerer):
Thanks for the patches and the deep analysis.

* I believe that even if it does not appear as a problem right now we should 
make the tests more robusts. 2 things that strike me are that the tables are 
not cleaned up between the tests and that {{forcedFlushes}} are used but the 
tests do not disable the automatic flushes.

I would change the {{cleanupData}} method into:
{code}
    private static void cleanupData()
    {
        stores().forEach(ColumnFamilyStore::truncateBlocking);
    }

    private static Stream<ColumnFamilyStore> stores()
    {
        Keyspace ks = Keyspace.open(KS_NAME);
        return ks.getMetadata().tables.stream().map(t -> 
ks.getColumnFamilyStore(t.name));
    }
{code}

and the {{cleanUp}} method into:
{code}
    @Before
    public void cleanUp()
    {
        cleanupData();
    }
{code} 
That would ensure that we clean all the tables between the different tests.

I would also add 
{{stores().forEach(ColumnFamilyStore::disableAutoCompaction);}} at the end of 
the {{loadSchema()}} method to ensure that there are no race condition with 
automatic compactions.

* Regarding the timestamp fix, would it not make sense to use some thing like:
{code}
    private static long timestamp = 0;

    private static long nextTimestamp()
    {
        timestamp += 1000;
        return timestamp;
    }
{code}
rather than specifying the timestamp for each call to {{loadData}}?

* Regarding {{testIndexMemtableSwitching}} problem in 3.11, it seems that the 
test has been changed as part of CASSANDRA-15867 and I am not convinced that 
this change was the correct one. If you roll back that change, do you still see 
the same issue?

> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to