You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "frankgh (via GitHub)" <gi...@apache.org> on 2025/08/01 22:45:53 UTC

[PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

frankgh opened a new pull request, #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243

   …integration tests
   
   Patch by Francisco Guerrero; reviewed by TBD for CASSSIDECAR-328


-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#discussion_r2249144245


##########
test-common/src/testFixtures/java/org/apache/cassandra/distributed/impl/CassandraCluster.java:
##########
@@ -70,9 +77,14 @@ public class CassandraCluster<I extends IInstance> implements IClusterExtension<
         return className.equals("org.apache.cassandra.utils.concurrent.Ref$OnLeak")
                || className.startsWith("org.apache.cassandra.metrics.RestorableMeter")
                || className.equals("org.apache.logging.slf4j.EventDataConverter")
-               || (className.startsWith("org.apache.cassandra.analytics.") && className.contains("BBHelper"));
+               || (className.startsWith("org.apache.cassandra.sidecar.") && className.contains("BBHelper"));

Review Comment:
   interesting...



##########
server/src/test/integration/org/apache/cassandra/sidecar/routes/tokenrange/JoiningMultiDCTest.java:
##########
@@ -209,13 +210,8 @@ private Map<String, Map<Range<BigInteger>, List<String>>> generateExpectedRangeD
         dc2Mapping.put(expectedRanges.get(12), Arrays.asList("127.0.0.4", "127.0.0.6", "127.0.0.2", "127.0.0.10",
                                                              "127.0.0.8"));
 
-        return new HashMap<String, Map<Range<BigInteger>, List<String>>>()
-        {
-            {
-                put("datacenter1", dc1Mapping);
-                put("datacenter2", dc2Mapping);
-            }
-        };
+        return Map.of("datacenter1", dc1Mapping,
+                      "datacenter2", dc2Mapping);
     }

Review Comment:
   👍 



##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/coordination/ClusterLeaseClaimTaskIntegrationTest.java:
##########
@@ -115,32 +111,35 @@ void setup()
     @AfterAll
     static void cleanup()
     {
-        TestResourceReaper.create().with(vertx).close();
+        getBlocking(vertx.close(), 10, TimeUnit.SECONDS, "Close vertx");

Review Comment:
   Sad to see moving away from `TestResourceReaper` :|



-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh merged PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243


-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#discussion_r2249328275


##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/coordination/ClusterLeaseClaimTaskIntegrationTest.java:
##########
@@ -115,32 +111,35 @@ void setup()
     @AfterAll
     static void cleanup()
     {
-        TestResourceReaper.create().with(vertx).close();
+        getBlocking(vertx.close(), 10, TimeUnit.SECONDS, "Close vertx");

Review Comment:
   There are opportunities to consolidate common test code in the test-common subproject. Maybe something to consider for future improvements 



-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#issuecomment-3146273709

   The CI duration has almost doubled due to the re-organization of the CI workflow. There are improvement can be done to again make unit test and dtest-jar build runs in parallel. It can be addressed 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.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#discussion_r2249013432


##########
server/src/test/integration/org/apache/cassandra/sidecar/db/SystemViewsDatabaseAccessorIntTest.java:
##########


Review Comment:
   migrated to integration-test tests



-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#discussion_r2248973686


##########
integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedExecutorNettyOptions.java:
##########


Review Comment:
   This class was duplicated in the integration-tests project as well as server. We have one single class in the test-common subproject



-- 
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: pr-unsubscribe@cassandra.apache.org

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

Re: [PR] CASSSIDECAR-328: Isolate dtest jar in its own classloader for server … [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #243:
URL: https://github.com/apache/cassandra-sidecar/pull/243#discussion_r2248971799


##########
.circleci/config.yml:
##########
@@ -103,7 +105,7 @@ jobs:
       - run:
           name: Build jvm dtest jars with jdk11
           command: |
-            CASSANDRA_USE_JDK11=true scripts/build-dtest-jars.sh

Review Comment:
   setting `CASSANDRA_USE_JDK11` is no longer necessary 



-- 
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: pr-unsubscribe@cassandra.apache.org

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