You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "lukasz-antoniak (via GitHub)" <gi...@apache.org> on 2024/05/24 11:25:46 UTC
[PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
lukasz-antoniak opened a new pull request, #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934
JIRA link: https://issues.apache.org/jira/browse/CASSANDRA-19635
PR contains necessary changes to run integration tests with Apache Cassandra 5.0-beta1. Developers can run the tests using:
```
$ mvn clean install -Dccm.version=5.0-beta1 -DtestJavaHome=/opt/java17/home -Ptest-jdk-17
```
Make sure you have latest CCM version installed locally and available in `$PATH`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613809842
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/SchemaChangeSynchronizer.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.api.testinfra.ccm;
+
+import java.util.concurrent.Semaphore;
+
+/**
+ * Running multiple parallel integration tests may fail due to query timeout when trying to apply
+ * several schema changes at once. Limit concurrently executed DDLs to 5.
+ */
+public class SchemaChangeSynchronizer {
+ private static final Semaphore lock = new Semaphore(5);
Review Comment:
Still wanted to preserve some parallelism in builds with lower C* versions. Also did not want to make this logic C* version dependent, just to simplify things. If you wish I am happy to change it to 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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#issuecomment-2137762352
Excellent, thanks for making those final changes so quickly @lukasz-antoniak! With your most recent work I believe we have all the code changes we need to get this merged!
The next step is to consolidate all of these changes down to a single commit. If it's useful we've provided some [fairly detailed instructions](https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2099192505) on how to do this in a comment on a prior PR. In your case the commit message should read something like the following:
```
CASSANDRA-19635: Run integration tests with C* 5.x
patch by Lukasz Antoniak; reviewed by Andy Tolbert, and Bret McGuire for CASSANDRA-19635
```
Feel free to ask questions if any part of that process isn't clear!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613810398
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
Review Comment:
Yes, this was a flaky test that I have fixed while running complete IT set multiple times a day :).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1619129238
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
+@BackendRequirement(
+ type = BackendType.CASSANDRA,
+ description = "SASI indexes broken in DSE. See InventoryITBase#BROKEN_SASI_VERSION.")
Review Comment:
This seems fine to me; it fits nicely with the pattern of each test being responsible for whatever customization it needs to operate.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1618668119
##########
integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java:
##########
@@ -254,6 +254,20 @@ public void should_get_virtual_metadata() {
+ " unit text,\n"
+ " PRIMARY KEY (keyspace_name, table_name, task_id)\n"
+ "); */",
+ // Cassandra 5.0
+ "/* VIRTUAL TABLE system_views.sstable_tasks (\n"
+ + " keyspace_name text,\n"
+ + " table_name text,\n"
+ + " task_id timeuuid,\n"
+ + " completion_ratio double,\n"
+ + " kind text,\n"
+ + " progress bigint,\n"
+ + " sstables int,\n"
+ + " target_directory text,\n"
+ + " total bigint,\n"
+ + " unit text,\n"
+ + " PRIMARY KEY (keyspace_name, table_name, task_id)\n"
+ + "); */",
Review Comment:
Fixed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1617673427
##########
integration-tests/src/test/java/com/datastax/oss/driver/core/metadata/SchemaIT.java:
##########
@@ -254,6 +254,20 @@ public void should_get_virtual_metadata() {
+ " unit text,\n"
+ " PRIMARY KEY (keyspace_name, table_name, task_id)\n"
+ "); */",
+ // Cassandra 5.0
+ "/* VIRTUAL TABLE system_views.sstable_tasks (\n"
+ + " keyspace_name text,\n"
+ + " table_name text,\n"
+ + " task_id timeuuid,\n"
+ + " completion_ratio double,\n"
+ + " kind text,\n"
+ + " progress bigint,\n"
+ + " sstables int,\n"
+ + " target_directory text,\n"
+ + " total bigint,\n"
+ + " unit text,\n"
+ + " PRIMARY KEY (keyspace_name, table_name, task_id)\n"
+ + "); */",
Review Comment:
Nit: the sequence of candidate sstable_tasks schemas goes DSE 6.8+/C* 4.0/C* 4.1. According to that sequence this block should appear last, after C* 4.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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613633816
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
Review Comment:
ah I see, there are LWT specific tests in here, so this comment is actually applicable.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613817782
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/BaseCcmRule.java:
##########
@@ -38,7 +38,7 @@ public abstract class BaseCcmRule extends CassandraResourceRule {
new Thread(
() -> {
try {
- ccmBridge.remove();
+ ccmBridge.close();
Review Comment:
I have made `CcmBridge#close()` dependent on `created` flag. Now we execute `ccm remove` only once if the cluster was previously created and not removed after that. If you run FT and log CCM command executions, you would see that we clean-up cluster twice (execute `ccm remove`):
1. In `BaseCcmRule#after()`.
2. As part of `AutoCloseable` and resource `close()` method.
Of course running `ccm remove` twice does not do any harm. IMO this fix is most about taste, so I can revert this back.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce merged PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#issuecomment-2135844487
Agree with @tolbertam ; well done @lukasz-antoniak! There are a few nits I'd like to see cleaned up but once those are in place I think I'm a 👍 as well.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#issuecomment-2145894890
Yup, this looks good to me. Thanks @lukasz-antoniak!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1615556869
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.java:
##########
@@ -199,7 +199,7 @@ public Version getCassandraVersion() {
private String getCcmVersionString(Version version) {
// for 4.0 pre-releases, the CCM version string needs to be "4.0-alpha1" or "4.0-alpha2"
// Version.toString() always adds a patch value, even if it's not specified when parsing.
- if (version.getMajor() == 4
Review Comment:
Good catch, modified.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "absurdfarce (via GitHub)" <gi...@apache.org>.
absurdfarce commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1617689369
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
+@BackendRequirement(
+ type = BackendType.CASSANDRA,
+ description = "SASI indexes broken in DSE. See InventoryITBase#BROKEN_SASI_VERSION.")
Review Comment:
I think this is too strong a constraint. IIRC SASI indexes were borked in a specific DSE release. [The commit](https://github.com/apache/cassandra-java-driver/commit/c94e39c6ef8b45259a38b7e4a9191f40472e193d) which introduced BROKEN_SASI_VERSION has a lot of useful context here, specifically the definition of [isSasiBroken()](https://github.com/apache/cassandra-java-driver/blob/4.x/integration-tests/src/test/java/com/datastax/oss/driver/mapper/InventoryITBase.java#L95-L99) which checks for an exact DSE version.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1614198117
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/BaseCcmRule.java:
##########
@@ -38,7 +38,7 @@ public abstract class BaseCcmRule extends CassandraResourceRule {
new Thread(
() -> {
try {
- ccmBridge.remove();
+ ccmBridge.close();
Review Comment:
Now that you've explained it, it makes sense to me, +1 for keeping 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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613635010
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/BaseCcmRule.java:
##########
@@ -38,7 +38,7 @@ public abstract class BaseCcmRule extends CassandraResourceRule {
new Thread(
() -> {
try {
- ccmBridge.remove();
+ ccmBridge.close();
Review Comment:
Why the change from `remove` to `close`? Possible this was changed to check directory outputs after but forgot to change back?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613624741
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
Review Comment:
comment isn't applicable right? Probably can remove it as I think the annotation description describes it well.
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/SchemaChangeSynchronizer.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.api.testinfra.ccm;
+
+import java.util.concurrent.Semaphore;
+
+/**
+ * Running multiple parallel integration tests may fail due to query timeout when trying to apply
+ * several schema changes at once. Limit concurrently executed DDLs to 5.
+ */
+public class SchemaChangeSynchronizer {
+ private static final Semaphore lock = new Semaphore(5);
Review Comment:
Any reason why we shouldn't just set this to 1?
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.java:
##########
@@ -199,7 +199,7 @@ public Version getCassandraVersion() {
private String getCcmVersionString(Version version) {
// for 4.0 pre-releases, the CCM version string needs to be "4.0-alpha1" or "4.0-alpha2"
// Version.toString() always adds a patch value, even if it's not specified when parsing.
- if (version.getMajor() == 4
Review Comment:
may be more future-proof to just say `>= 4` here as likely to have same problem with future releases.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1613633816
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
Review Comment:
ah I see, there are LWT specific tests in here, so this comment is actually applicable (`should_delete_if_exists_reactive` for example)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "tolbertam (via GitHub)" <gi...@apache.org>.
tolbertam commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1614216360
##########
test-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/SchemaChangeSynchronizer.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.api.testinfra.ccm;
+
+import java.util.concurrent.Semaphore;
+
+/**
+ * Running multiple parallel integration tests may fail due to query timeout when trying to apply
+ * several schema changes at once. Limit concurrently executed DDLs to 5.
+ */
+public class SchemaChangeSynchronizer {
+ private static final Semaphore lock = new Semaphore(5);
Review Comment:
I think that concurrent schema changes happen to 'work' in older versions is probably more by luck than anything. I think it likely would not slow down things noticeable if we set it to 1, and its a lot more appropriate (have seen too many incidents caused by concurrent schema changes, its always nice to enforce 1 at a time). That said, i'm cool with merging and leaving it at 5 as well, since it's better than what it does currently :+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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org
Re: [PR] CASSANDRA-19635 - Run integration tests with C* 5.x [cassandra-java-driver]
Posted by "lukasz-antoniak (via GitHub)" <gi...@apache.org>.
lukasz-antoniak commented on code in PR #1934:
URL: https://github.com/apache/cassandra-java-driver/pull/1934#discussion_r1618670454
##########
integration-tests/src/test/java/com/datastax/oss/driver/mapper/DeleteReactiveIT.java:
##########
@@ -34,23 +34,27 @@
import com.datastax.oss.driver.api.mapper.annotations.Mapper;
import com.datastax.oss.driver.api.mapper.annotations.Select;
import com.datastax.oss.driver.api.mapper.entity.saving.NullSavingStrategy;
-import com.datastax.oss.driver.api.testinfra.ccm.CcmRule;
+import com.datastax.oss.driver.api.testinfra.ccm.CustomCcmRule;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendRequirement;
+import com.datastax.oss.driver.api.testinfra.requirement.BackendType;
import com.datastax.oss.driver.api.testinfra.session.SessionRule;
-import com.datastax.oss.driver.categories.ParallelizableTests;
import io.reactivex.Flowable;
import java.util.UUID;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
-import org.junit.experimental.categories.Category;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
-@Category(ParallelizableTests.class)
+// Do not run LWT tests in parallel because they may interfere. Tests operate on the same row.
+@BackendRequirement(
+ type = BackendType.CASSANDRA,
+ description = "SASI indexes broken in DSE. See InventoryITBase#BROKEN_SASI_VERSION.")
Review Comment:
I had a wrong impression that there are more DSE versions that are actually broken. Hereby integration tests for deletion of entities, we do not want to run in parallel, because they may interfere one with another. Shared `CcmBridge` can be used only with `@Category(ParallelizableTests.class)`, which we do not want. `CustomCcmRule` is mostly used together with `@BackendRequirement`. I had a problem with this test running agains DSE, because `enable_sasi_indexes` is an invalid configuration parameter.
So to make this test work for C*, I had to set `enable_sasi_indexes = true`, but not for DSE.
After further debugging, I have found the root cause - `DefaultCcmBridgeBuilderCustomizer` is not applied to `CustomCcmRule`. I shall not reuse `DefaultCcmBridgeBuilderCustomizer`, because it adds other parameters that causes other integration tests to break (once using `CustomCcmRule`). In the end, I have implemented a dedicated `configureCcm` method inside the tests. `enable_sasi_indexes` is required for one delete statement to succeed inside this test class, so I think we can leave configuration local.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org