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/09/07 12:33:45 UTC

[GitHub] [cassandra] adelapena commented on a change in pull request #1187: CASSANDRA-16918: Open java driver connections in CQLTester in a lazy way

adelapena commented on a change in pull request #1187:
URL: https://github.com/apache/cassandra/pull/1187#discussion_r703463783



##########
File path: test/unit/org/apache/cassandra/cql3/CQLTester.java
##########
@@ -462,54 +466,45 @@ protected static void reinitializeNetwork(Consumer<Cluster.Builder> clusterConfi
         clusters.clear();
         sessions.clear();
 
-        initializeNetwork(server -> {}, clusterConfigurator);
+        clusterBuilderConfigurator = clusterConfigurator;
+
+        startServer(serverConfigurator);
     }
 
-    private static void initializeNetwork(Consumer<Server.Builder> decorator, Consumer<Cluster.Builder> clusterConfigurator)
+    private static void startServer(Consumer<Server.Builder> decorator)
     {
-        startServer(decorator);
-
-        for (ProtocolVersion version : PROTOCOL_VERSIONS)
-        {
-            if (clusters.containsKey(version))
-                continue;
+        Server.Builder serverBuilder = new Server.Builder().withHost(nativeAddr).withPort(nativePort);
+        decorator.accept(serverBuilder);
+        server = serverBuilder.build();
+        ClientMetrics.instance.init(Collections.singleton(server));
+        server.start();
+    }
 
-            SocketOptions socketOptions = new SocketOptions()
-                                          .setConnectTimeoutMillis(Integer.getInteger("cassandra.test.driver.connection_timeout_ms", DEFAULT_CONNECT_TIMEOUT_MILLIS)) // default is 5000
-                                          .setReadTimeoutMillis(Integer.getInteger("cassandra.test.driver.read_timeout_ms", DEFAULT_READ_TIMEOUT_MILLIS)); // default is 12000
+    private static Cluster initClientCluster(ProtocolVersion version)
+    {
+        SocketOptions socketOptions = 
+                new SocketOptions().setConnectTimeoutMillis(Integer.getInteger("cassandra.test.driver.connection_timeout_ms",
+                                                            DEFAULT_CONNECT_TIMEOUT_MILLIS)) // default is 5000
+                                   .setReadTimeoutMillis(Integer.getInteger("cassandra.test.driver.read_timeout_ms",
+                                                         DEFAULT_READ_TIMEOUT_MILLIS)); // default is 12000
 
-            logger.info("Timeouts: {} / {}", socketOptions.getConnectTimeoutMillis(), socketOptions.getReadTimeoutMillis());
+        logger.info("Timeouts: {} / {}", socketOptions.getConnectTimeoutMillis(), socketOptions.getReadTimeoutMillis());
 
-            Cluster.Builder builder = Cluster.builder()
-                                             .withoutJMXReporting()
-                                             .addContactPoints(nativeAddr)
-                                             .withClusterName("Test Cluster")
-                                             .withPort(nativePort)
-                                             .withSocketOptions(socketOptions);
+        Cluster.Builder builder = Cluster.builder().withoutJMXReporting().addContactPoints(nativeAddr)
+                .withClusterName("Test Cluster").withPort(nativePort).withSocketOptions(socketOptions);

Review comment:
       Nit: maybe the previous on-column indentation was a bit easier to read:
   ```suggestion
           Cluster.Builder builder = Cluster.builder()
                                            .withoutJMXReporting()
                                            .addContactPoints(nativeAddr)
                                            .withClusterName("Test Cluster")
                                            .withPort(nativePort)
                                            .withSocketOptions(socketOptions);
   ```

##########
File path: test/unit/org/apache/cassandra/cql3/CQLTester.java
##########
@@ -462,54 +466,45 @@ protected static void reinitializeNetwork(Consumer<Cluster.Builder> clusterConfi
         clusters.clear();
         sessions.clear();
 
-        initializeNetwork(server -> {}, clusterConfigurator);
+        clusterBuilderConfigurator = clusterConfigurator;
+
+        startServer(serverConfigurator);
     }
 
-    private static void initializeNetwork(Consumer<Server.Builder> decorator, Consumer<Cluster.Builder> clusterConfigurator)
+    private static void startServer(Consumer<Server.Builder> decorator)
     {
-        startServer(decorator);
-
-        for (ProtocolVersion version : PROTOCOL_VERSIONS)
-        {
-            if (clusters.containsKey(version))
-                continue;
+        Server.Builder serverBuilder = new Server.Builder().withHost(nativeAddr).withPort(nativePort);
+        decorator.accept(serverBuilder);
+        server = serverBuilder.build();
+        ClientMetrics.instance.init(Collections.singleton(server));
+        server.start();
+    }
 
-            SocketOptions socketOptions = new SocketOptions()
-                                          .setConnectTimeoutMillis(Integer.getInteger("cassandra.test.driver.connection_timeout_ms", DEFAULT_CONNECT_TIMEOUT_MILLIS)) // default is 5000
-                                          .setReadTimeoutMillis(Integer.getInteger("cassandra.test.driver.read_timeout_ms", DEFAULT_READ_TIMEOUT_MILLIS)); // default is 12000
+    private static Cluster initClientCluster(ProtocolVersion version)
+    {
+        SocketOptions socketOptions = 
+                new SocketOptions().setConnectTimeoutMillis(Integer.getInteger("cassandra.test.driver.connection_timeout_ms",
+                                                            DEFAULT_CONNECT_TIMEOUT_MILLIS)) // default is 5000
+                                   .setReadTimeoutMillis(Integer.getInteger("cassandra.test.driver.read_timeout_ms",
+                                                         DEFAULT_READ_TIMEOUT_MILLIS)); // default is 12000

Review comment:
       Nit: aligning
   ```suggestion
           SocketOptions socketOptions = 
                   new SocketOptions().setConnectTimeoutMillis(Integer.getInteger("cassandra.test.driver.connection_timeout_ms", 
                                                                                  DEFAULT_CONNECT_TIMEOUT_MILLIS)) // default is 5000
                                      .setReadTimeoutMillis(Integer.getInteger("cassandra.test.driver.read_timeout_ms",
                                                                               DEFAULT_READ_TIMEOUT_MILLIS)); // default is 12000
   ```

##########
File path: test/unit/org/apache/cassandra/cql3/CQLTester.java
##########
@@ -125,7 +125,9 @@
     protected static final InetAddress nativeAddr;
     protected static final Set<InetAddressAndPort> remoteAddrs = new HashSet<>();
     private static final Map<ProtocolVersion, Cluster> clusters = new HashMap<>();
-    protected static final Map<ProtocolVersion, Session> sessions = new HashMap<>();
+    private static final Map<ProtocolVersion, Session> sessions = new HashMap<>();

Review comment:
       I was going to say that we could use `EnumMap<>(ProtocolVersion.class)` here and in `Clusters`, but I guess that the plan is to convert the keys in pairs of user and protocol version in a short while, so I guess this way is fine.




-- 
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