You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/04/14 09:18:47 UTC
[GitHub] [cassandra] bereng opened a new pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
bereng opened a new pull request #966:
URL: https://github.com/apache/cassandra/pull/966
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615735441
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -558,9 +561,17 @@ public void indexStatementsWithConditions() throws Throwable
@Test
public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
{
- String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
+ String selectBuiltIndexesQuery = String.format("SELECT table_name, index_name FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete. Warn: When we used to run tests in parallel there
+ // could also be cross test class talk and have other indices pop up here.
+ Awaitility.await()
+ .atMost(1, TimeUnit.MINUTES)
+ .pollDelay(1, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
+
UntypedResultSet rs = execute(selectBuiltIndexesQuery);
Review comment:
Saving the `initialSize` is part of the fix introduced by CASSANDRA-15565 that we can get rid of. The checks after dropping the index can be simplified to just verifying that the `IndexInfo` table is empty, as suggested [here](https://github.com/adelapena/cassandra/blob/8cf7145b8437f616e8085169cc3829197961031c/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java#L567-L584).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616731871
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -26,6 +27,7 @@
import com.google.common.collect.*;
import org.junit.Test;
+import org.apache.cassandra.Util;
Review comment:
Nit: this unused import is still around, it can be removed during commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616599979
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -26,6 +27,7 @@
import com.google.common.collect.*;
import org.junit.Test;
+import org.apache.cassandra.Util;
Review comment:
Nit: unused import
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616710887
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,33 +564,35 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete. Warn: When we used to run tests in parallel there
+ // could also be cross test class talk and have other indices pop up here.
+ Awaitility.await()
+ .atMost(1, TimeUnit.MINUTES)
+ .pollDelay(1, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
+
UntypedResultSet rs = execute(selectBuiltIndexesQuery);
Review comment:
Right. I missed that warning apologies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615737739
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
+ Util.spinAssertEquals(0, () -> {
+ try
+ {
+ return execute(selectBuiltIndexesQuery).size();
+ }
+ catch(Throwable e)
+ {
+ fail(e.getMessage());
+ }
+ return -1;
+ }, 60);
Review comment:
That's what I was thinking. Unless Thread.yield() provides some advantage bc it's more aggressive we could just replace that method contents with Awaitility...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615735441
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -558,9 +561,17 @@ public void indexStatementsWithConditions() throws Throwable
@Test
public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
{
- String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
+ String selectBuiltIndexesQuery = String.format("SELECT table_name, index_name FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete. Warn: When we used to run tests in parallel there
+ // could also be cross test class talk and have other indices pop up here.
+ Awaitility.await()
+ .atMost(1, TimeUnit.MINUTES)
+ .pollDelay(1, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
+
UntypedResultSet rs = execute(selectBuiltIndexesQuery);
Review comment:
Saving the `initialSize` is part of the fix introduced by CASSANDRA-15565 that we can get rid of. The checks after dropping the int can be simplified to just verifying that the `IndexInfo` table is empty, as suggested [here](https://github.com/adelapena/cassandra/blob/8cf7145b8437f616e8085169cc3829197961031c/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java#L567-L584).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615545520
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
Review comment:
I'm sort of -1. Junit test methods _by definition_ should be 100% isolated. Junit itself goes to great lenghts to make sure each test case is isolated as much as possible. CASSANDRA-16595 is actually solving _a bug_ where we broke junit rules. I can add for completeness but I'll reword it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616342454
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -558,9 +561,17 @@ public void indexStatementsWithConditions() throws Throwable
@Test
public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
{
- String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
+ String selectBuiltIndexesQuery = String.format("SELECT table_name, index_name FROM %s.\"%s\"",
Review comment:
Nah I see your point. +1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r614749297
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
Review comment:
Maybe we could add a note about parallel runners, in case they come back in the future?
```suggestion
// Wait for any background index clearing tasks to complete,
// knowing that there aren't any other tests running parallely
```
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
+ Util.spinAssertEquals(0, () -> {
+ try
+ {
+ return execute(selectBuiltIndexesQuery).size();
+ }
+ catch(Throwable e)
+ {
+ fail(e.getMessage());
+ }
+ return -1;
+ }, 60);
Review comment:
We can use the briefer and more flexible `Awaitility.await()` method, that doesn't require the try-catch block and allows us to specify a poll delay:
```suggestion
await().atMost(1, MINUTES)
.pollDelay(1, SECONDS)
.untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615731001
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
+ Util.spinAssertEquals(0, () -> {
+ try
+ {
+ return execute(selectBuiltIndexesQuery).size();
+ }
+ catch(Throwable e)
+ {
+ fail(e.getMessage());
+ }
+ return -1;
+ }, 60);
Review comment:
I don't know, I think the dependency for the more sophisticated `Awaitability` was added quite recently by CASSANDRA-15677. Maybe it was because `spinAssertEquals` is not visible from dtests? Perhaps we should consider getting rid of one of them in a separate ticket?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615547298
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
+ Util.spinAssertEquals(0, () -> {
+ try
+ {
+ return execute(selectBuiltIndexesQuery).size();
+ }
+ catch(Throwable e)
+ {
+ fail(e.getMessage());
+ }
+ return -1;
+ }, 60);
Review comment:
So what's the point of having 2 methods that do the same? Is it bc one uses Thread.yield()?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r617229578
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
+ Util.spinAssertEquals(0, () -> {
+ try
+ {
+ return execute(selectBuiltIndexesQuery).size();
+ }
+ catch(Throwable e)
+ {
+ fail(e.getMessage());
+ }
+ return -1;
+ }, 60);
Review comment:
https://issues.apache.org/jira/browse/CASSANDRA-16621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616597100
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,33 +564,35 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete. Warn: When we used to run tests in parallel there
+ // could also be cross test class talk and have other indices pop up here.
+ Awaitility.await()
+ .atMost(1, TimeUnit.MINUTES)
+ .pollDelay(1, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
+
UntypedResultSet rs = execute(selectBuiltIndexesQuery);
- int initialSize = rs.size();
String indexName = "build_remove_test_idx";
String tableName = createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))");
createIndex(String.format("CREATE INDEX %s ON %%s(c)", indexName));
waitForIndex(KEYSPACE, tableName, indexName);
+
// check that there are no other rows in the built indexes table
- rs = execute(selectBuiltIndexesQuery);
- int sizeAfterBuild = rs.size();
- assertRowsIgnoringOrderAndExtra(rs, row(KEYSPACE, indexName, null));
+ assertRows(execute(selectBuiltIndexesQuery), row(KEYSPACE, indexName, null));
// rebuild the index and verify the built status table
getCurrentColumnFamilyStore().rebuildSecondaryIndex(indexName);
waitForIndex(KEYSPACE, tableName, indexName);
// check that there are no other rows in the built indexes table
- rs = execute(selectBuiltIndexesQuery);
- assertEquals(sizeAfterBuild, rs.size());
- assertRowsIgnoringOrderAndExtra(rs, row(KEYSPACE, indexName, null));
+ assertRows(execute(selectBuiltIndexesQuery), row(KEYSPACE, indexName, null));
// check that dropping the index removes it from the built indexes table
dropIndex("DROP INDEX %s." + indexName);
rs = execute(selectBuiltIndexesQuery);
Review comment:
Nit: not needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615545520
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,6 +562,20 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete
Review comment:
I'm sort of -1. Junit test methods _by definition_ should be 100% isolated. Junit itself goes to great lenghts to make sure each test case is isolated as much as possible. CASSANDRA-16595 is actually solving _a bug_ where we broke junit rules. I can it add for completeness but I'll reword it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #966:
URL: https://github.com/apache/cassandra/pull/966#issuecomment-824545243
- CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/257/workflows/63da7c04-7116-45e0-9986-3ccb94980fee)
- CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/257/workflows/121e09d4-e2b8-4320-a373-f7fed5a95ece)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #966:
URL: https://github.com/apache/cassandra/pull/966#issuecomment-823072166
Latest done and 200 runs ok @adelapena
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r615742631
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -558,9 +561,17 @@ public void indexStatementsWithConditions() throws Throwable
@Test
public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
{
- String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
+ String selectBuiltIndexesQuery = String.format("SELECT table_name, index_name FROM %s.\"%s\"",
Review comment:
I'd say that checking that the index build doesn't write the legacy `IndexInfo.value` column had some value, but I'm not against the simplification if you prefer it this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616816158
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -26,6 +27,7 @@
import com.google.common.collect.*;
import org.junit.Test;
+import org.apache.cassandra.Util;
Review comment:
I _did_ delete that guy :thinking: I'll push along with 3.11 later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #966:
URL: https://github.com/apache/cassandra/pull/966#discussion_r616596846
##########
File path: test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
##########
@@ -561,33 +564,35 @@ public void indexCorrectlyMarkedAsBuildAndRemoved() throws Throwable
String selectBuiltIndexesQuery = String.format("SELECT * FROM %s.\"%s\"",
SchemaConstants.SYSTEM_KEYSPACE_NAME,
SystemKeyspace.BUILT_INDEXES);
+
+ // Wait for any background index clearing tasks to complete. Warn: When we used to run tests in parallel there
+ // could also be cross test class talk and have other indices pop up here.
+ Awaitility.await()
+ .atMost(1, TimeUnit.MINUTES)
+ .pollDelay(1, TimeUnit.SECONDS)
+ .untilAsserted(() -> assertRows(execute(selectBuiltIndexesQuery)));
+
UntypedResultSet rs = execute(selectBuiltIndexesQuery);
Review comment:
Nit: this is not used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng commented on pull request #966:
URL: https://github.com/apache/cassandra/pull/966#issuecomment-819417674
- CI [j11](https://app.circleci.com/pipelines/github/bereng/cassandra/250/workflows/6da983ee-dc33-4c9b-bcad-7e793e8cc946) unrelated failures
- CI [j8](https://app.circleci.com/pipelines/github/bereng/cassandra/250/workflows/79e5becb-40d6-47d1-9b6d-7f2b0c7b6008) unrelated failures
- 100 multiplexer runs successful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng closed pull request #966: CASSANDRA-16601 Flaky CassandraIndexTest
Posted by GitBox <gi...@apache.org>.
bereng closed pull request #966:
URL: https://github.com/apache/cassandra/pull/966
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org