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 2023/10/16 22:51:09 UTC

[PR] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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

   Vertx 4.4.6 brings two features that we integrate into Sidecar.
   
   1. Hot reloading of SSL certificates. This allows a running cluster to reload certificates without having to restart the service.
   
   2. Traffic shaping options. This allows to introduce protections for the service. It allows configuring ingress/egress limits.
   
   Additionally, this patch introduces the SidecarServerEvents messaging. It leverages vertx's EventBus to publish and consume messages when server starts, server stops, on CQL connection ready, or CQL disconnection, and when all CQL connections are ready.
   
   Patch for [CASSANDRASC-77](https://issues.apache.org/jira/browse/CASSANDRASC-77)


-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/CassandraSidecarDaemon.java:
##########
@@ -18,177 +18,75 @@
 
 package org.apache.cassandra.sidecar;
 
-import java.io.PrintStream;
-import java.util.ArrayList;
-import java.util.List;
+import java.net.URI;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.concurrent.TimeUnit;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.inject.Guice;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import io.vertx.core.CompositeFuture;
-import io.vertx.core.Future;
-import io.vertx.core.Vertx;
-import io.vertx.core.http.HttpServer;
-import org.apache.cassandra.sidecar.cluster.InstancesConfig;
-import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
-import org.apache.cassandra.sidecar.config.ServiceConfiguration;
-import org.apache.cassandra.sidecar.config.SidecarConfiguration;
-import org.apache.cassandra.sidecar.config.SslConfiguration;
-import org.apache.cassandra.sidecar.utils.SslUtils;
+import org.apache.cassandra.sidecar.server.MainModule;
+import org.apache.cassandra.sidecar.server.Server;
 
 /**
  * Main class for initiating the Cassandra sidecar
  * Note: remember to start and stop all delegates of instances
  */
-@Singleton
 public class CassandraSidecarDaemon
 {
-    private static final Logger logger = LoggerFactory.getLogger(CassandraSidecarDaemon.class);
-    private final Vertx vertx;
-    private final HttpServer server;
-    private final SidecarConfiguration config;
-    private final InstancesConfig instancesConfig;
-    private final ExecutorPools executorPools;
-
-    private long healthCheckTimerId;
-
-    @Inject
-    public CassandraSidecarDaemon(Vertx vertx,

Review Comment:
   this code is now captured under server/Server.java



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/snapshots/SnapshotPathBuilder.java:
##########
@@ -140,79 +139,77 @@ public Future<List<SnapshotFile>> listSnapshotDirectory(String snapshotDirectory
               logger.debug("Found {} files in snapshot directory '{}'", list.size(), snapshotDirectory);
 
               // Prepare futures to get properties for all the files from listing the snapshot directory
-              //noinspection rawtypes
-              List<Future> futures = list.stream()
-                                         .map(fs::props)
-                                         .collect(Collectors.toList());
-
-              CompositeFuture.all(futures)
-                             .onFailure(cause -> {
-                                 logger.debug("Failed to get FileProps", cause);
-                                 promise.fail(cause);
-                             })
-                             .onSuccess(ar -> {
-
-                                 // Create a pair of path/fileProps for every regular file
-                                 List<SnapshotFile> snapshotList =
-                                 IntStream.range(0, list.size())
-                                          .filter(i -> ar.<FileProps>resultAt(i).isRegularFile())
-                                          .mapToObj(i -> {
-                                              long size = ar.<FileProps>resultAt(i).size();
-                                              return new SnapshotFile(list.get(i),
-                                                                      size);
-                                          })
-                                          .collect(Collectors.toList());
-
-
-                                 if (!includeSecondaryIndexFiles)
-                                 {
-                                     // We are done if we don't include secondary index files
-                                     promise.complete(snapshotList);
-                                     return;
-                                 }
-
-                                 // Find index directories and prepare futures listing the snapshot directory
-                                 //noinspection rawtypes
-                                 List<Future> idxListFutures =
-                                 IntStream.range(0, list.size())
-                                          .filter(i -> {
-                                              if (ar.<FileProps>resultAt(i).isDirectory())
-                                              {
-                                                  Path path = Paths.get(list.get(i));
-                                                  int count = path.getNameCount();
-                                                  return count > 0
-                                                         && path.getName(count - 1)
-                                                                .toString()
-                                                                .startsWith(".");
-                                              }
-                                              return false;
-                                          })
-                                          .mapToObj(i -> listSnapshotDirectory(list.get(i), false))
-                                          .collect(Collectors.toList());
-                                 if (idxListFutures.isEmpty())
-                                 {
-                                     // If there are no secondary index directories we are done
-                                     promise.complete(snapshotList);
-                                     return;
-                                 }
-                                 logger.debug("Found {} index directories in the '{}' snapshot",
-                                              idxListFutures.size(), snapshotDirectory);
-                                 // if we have index directories, list them all
-                                 CompositeFuture.all(idxListFutures)
-                                                .onFailure(promise::fail)
-                                                .onSuccess(idx -> {
-                                                    //noinspection unchecked
-                                                    List<SnapshotFile> idxPropList =
-                                                    idx.list()
-                                                       .stream()
-                                                       .flatMap(l -> ((List<SnapshotFile>) l).stream())
-                                                       .collect(Collectors.toList());
-
-                                                    // aggregate the results and return the full list
-                                                    snapshotList.addAll(idxPropList);
-                                                    promise.complete(snapshotList);
-                                                });
-                             });
+              List<Future<FileProps>> futures = list.stream()
+                                                    .map(fs::props)
+                                                    .collect(Collectors.toList());
+
+              Future.all(futures)

Review Comment:
   CompositeFuture was deprecated. Using Future now which now supports typed futures



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/config/ServiceConfiguration.java:
##########
@@ -33,11 +36,33 @@ public interface ServiceConfiguration
      */
     String host();
 
+    /**
+     * Returns a list of interfaces where the Sidecar process will bind and listen for connections. Defaults to
+     * the configured {@link #host()}.
+     *
+     * @return a list of interfaces where Sidecar will listen
+     */
+    default List<String> listenInterfaces()
+    {
+        return Collections.singletonList(Objects.requireNonNull(host(), "host must be provided"));
+    }
+
     /**
      * @return Sidecar's HTTP REST API port
      */
     int port();
 
+    /**
+     * Returns a list of ports where the Sidecar process will bind and listen for connections. Defaults to
+     * the configured {@link #port()}
+     *
+     * @return a list of ports where Sidecar will listen
+     */
+    default List<Integer> ports()
+    {
+        return Collections.singletonList(port());

Review Comment:
   makes sense, I will modify the API



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
spotbugs-exclude.xml:
##########
@@ -48,7 +48,7 @@
 
     <!-- Ignore GC call in CassandraTestTemplate as we want to GC after cluster shutdown -->
     <Match>
-        <Class name="org.apache.cassandra.sidecar.testing.CassandraTestTemplate" />
+        <Class name="org.apache.cassandra.sidecar.test.CassandraTestTemplate" />

Review Comment:
   when using `testing` IntelliJ was not able to locate this class, which caused a lot of issues in IDE. So I decided to move the package from testing -> test. Now everything is good it seems



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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

   Closed via https://github.com/apache/cassandra-sidecar/commit/8045f8eedad4510ecbf66bb739d464eec741aced


-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/server/SidecarServerEvents.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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 org.apache.cassandra.sidecar.server;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import io.vertx.core.Vertx;
+import io.vertx.core.eventbus.EventBus;
+import io.vertx.core.eventbus.MessageConsumer;
+import io.vertx.core.json.JsonArray;
+import io.vertx.core.json.JsonObject;
+import org.apache.cassandra.sidecar.cluster.InstancesConfig;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+
+/**
+ * Defines the {@link io.vertx.core.eventbus.EventBus} addresses where different notifications will be published
+ * during Sidecar startup/shutdown, as well as CQL connection availability.
+ *
+ * <p>The messages can be published multiple times depending on whether Sidecar is started or stopped
+ * during the lifetime of the application. Implementing consumers will need to deal with this expectation
+ * internally.
+ * <p>
+ * The expectation is that:
+ * <ul>
+ * <li>{@link #ON_SERVER_START} will happen first
+ * <li>{@link #ON_SERVER_STOP} can happen before {@link #ON_ALL_CASSANDRA_CQL_READY}
+ * <li>{@link #ON_SERVER_START} can only happen for any subsequent calls only after a {@link #ON_SERVER_STOP} message
+ * <li>{@link #ON_ALL_CASSANDRA_CQL_READY} might never happen
+ * <li>{@link #ON_CASSANDRA_CQL_READY} can be called multiple times with different cassandraInstanceId values
+ * <li>{@link #ON_CASSANDRA_CQL_DISCONNECTED} can be called multiple times with different cassandraInstanceId values
+ * </ul>
+ * <p>
+ * However, implementers should choose to implement methods assuming no guarantees to the event sequence.
+ */
+@Singleton
+public class SidecarServerEvents
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger(SidecarServerEvents.class);
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where server start events will be published. Server start
+     * will be published whenever Sidecar has successfully started and is ready listening for requests.
+     */
+    public static final String ON_SERVER_START = SidecarServerEvents.class.getName() + ".ON_SERVER_START";
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where server stop/shutdown events will be published.
+     * Server stop events will be published whenever Sidecar is stopping or shutting down.
+     */
+    public static final String ON_SERVER_STOP = SidecarServerEvents.class.getName() + ".ON_SERVER_STOP";
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where events will be published when a CQL connection for
+     * a given instance has been established. The instance identifier will be passed as part of the message.
+     */
+    public static final String ON_CASSANDRA_CQL_READY = SidecarServerEvents.class.getName() + ".ON_CASSANDRA_CQL_READY";

Review Comment:
   makes sense, refactored 



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
@@ -20,7 +20,8 @@
 
 import java.util.List;
 
-import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
+import org.apache.cassandra.sidecar.utils.CassandraAdapterDelegate;

Review Comment:
   yeah, agreed



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/config/yaml/ServiceConfigurationImpl.java:
##########
@@ -88,117 +107,45 @@ public class ServiceConfigurationImpl implements ServiceConfiguration
     @JsonProperty(value = WORKER_POOLS_PROPERTY, required = true)
     protected final Map<String, ? extends WorkerPoolConfiguration> workerPoolsConfiguration;
 
-    @JsonProperty("jmx")
+    @JsonProperty(value = JMX_PROPERTY)
     protected final JmxConfiguration jmxConfiguration;
 
-    public ServiceConfigurationImpl()
-    {
-        this(DEFAULT_HOST,
-             DEFAULT_PORT,
-             DEFAULT_REQUEST_IDLE_TIMEOUT_MILLIS,
-             DEFAULT_REQUEST_TIMEOUT_MILLIS,
-             DEFAULT_ALLOWABLE_SKEW_IN_MINUTES,
-             new ThrottleConfigurationImpl(),
-             new SSTableUploadConfigurationImpl(),
-             new SSTableImportConfigurationImpl(),
-             DEFAULT_WORKER_POOLS_CONFIGURATION,
-             new JmxConfigurationImpl());
-    }
-
-    public ServiceConfigurationImpl(SSTableImportConfiguration ssTableImportConfiguration)
-    {
-        this(DEFAULT_HOST,
-             DEFAULT_PORT,
-             DEFAULT_REQUEST_IDLE_TIMEOUT_MILLIS,
-             DEFAULT_REQUEST_TIMEOUT_MILLIS,
-             DEFAULT_ALLOWABLE_SKEW_IN_MINUTES,
-             new ThrottleConfigurationImpl(),
-             new SSTableUploadConfigurationImpl(),
-             ssTableImportConfiguration,
-             DEFAULT_WORKER_POOLS_CONFIGURATION,
-             new JmxConfigurationImpl());
-    }
-
-    public ServiceConfigurationImpl(String host,
-                                    ThrottleConfiguration throttleConfiguration,
-                                    SSTableUploadConfiguration ssTableUploadConfiguration,
-                                    JmxConfiguration jmxConfiguration)
-    {
-        this(host,
-             DEFAULT_PORT,
-             DEFAULT_REQUEST_IDLE_TIMEOUT_MILLIS,
-             DEFAULT_REQUEST_TIMEOUT_MILLIS,
-             DEFAULT_ALLOWABLE_SKEW_IN_MINUTES,
-             throttleConfiguration,
-             ssTableUploadConfiguration,
-             new SSTableImportConfigurationImpl(),
-             DEFAULT_WORKER_POOLS_CONFIGURATION,
-             jmxConfiguration);
-    }
-
-    public ServiceConfigurationImpl(int requestIdleTimeoutMillis,
-                                    long requestTimeoutMillis,
-                                    SSTableUploadConfiguration ssTableUploadConfiguration)
-    {
+    @JsonProperty(value = TRAFFIC_SHAPING_PROPERTY)
+    protected final TrafficShapingConfiguration trafficShapingConfiguration;
 
-        this(DEFAULT_HOST,
-             DEFAULT_PORT,
-             requestIdleTimeoutMillis,
-             requestTimeoutMillis,
-             DEFAULT_ALLOWABLE_SKEW_IN_MINUTES,
-             new ThrottleConfigurationImpl(),
-             ssTableUploadConfiguration,
-             new SSTableImportConfigurationImpl(),
-             DEFAULT_WORKER_POOLS_CONFIGURATION,
-             new JmxConfigurationImpl());
-    }
-
-    public ServiceConfigurationImpl(String host,
-                                    int port,
-                                    int requestIdleTimeoutMillis,
-                                    long requestTimeoutMillis,
-                                    int allowableSkewInMinutes,
-                                    ThrottleConfiguration throttleConfiguration,
-                                    SSTableUploadConfiguration ssTableUploadConfiguration,
-                                    SSTableImportConfiguration ssTableImportConfiguration,
-                                    Map<String, ? extends WorkerPoolConfiguration> workerPoolsConfiguration,
-                                    JmxConfiguration jmxConfiguration)
+    /**
+     * Constructs a new {@link ServiceConfigurationImpl} with the default values
+     */
+    public ServiceConfigurationImpl()
     {
-        this.host = host;
-        this.port = port;
-        this.requestIdleTimeoutMillis = requestIdleTimeoutMillis;
-        this.requestTimeoutMillis = requestTimeoutMillis;
-        this.allowableSkewInMinutes = allowableSkewInMinutes;
-        this.throttleConfiguration = throttleConfiguration;
-        this.ssTableUploadConfiguration = ssTableUploadConfiguration;
-        this.ssTableImportConfiguration = ssTableImportConfiguration;
-        this.jmxConfiguration = jmxConfiguration;
-        if (workerPoolsConfiguration == null || workerPoolsConfiguration.isEmpty())
-        {
-            this.workerPoolsConfiguration = DEFAULT_WORKER_POOLS_CONFIGURATION;
-        }
-        else
-        {
-            this.workerPoolsConfiguration = workerPoolsConfiguration;
-        }
+        this(builder());
     }
 
-    public ServiceConfigurationImpl(String host)
+    /**
+     * Constructs a new {@link ServiceConfigurationImpl} with the configured {@link Builder}
+     *
+     * @param builder the builder object
+     */
+    protected ServiceConfigurationImpl(Builder builder)

Review Comment:
   this is another case of exploding number of constructors. So I decided to simplify with a builder



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/concurrent/ExecutorPools.java:
##########
@@ -119,9 +120,45 @@ public long setPeriodic(long delay, Handler<Long> handler)
         public long setPeriodic(long delay, Handler<Long> handler, boolean ordered)
         {
             return vertx.setPeriodic(delay,
-                                     id -> workerExecutor.executeBlocking(promise -> {
+                                     id -> workerExecutor.executeBlocking(() -> {
                                          handler.handle(id);
-                                         promise.complete();
+                                         return id;
+                                     }, ordered));
+        }
+
+        /**
+         * Set a periodic timer to fire every {@code delay} milliseconds with initial delay, at which point
+         * {@code handler} will be called with the id of the timer.
+         *
+         * @param initialDelay the initial delay in milliseconds
+         * @param delay        the delay in milliseconds, after which the timer will fire
+         * @param handler      the handler that will be called with the timer ID when the timer fires
+         * @return the unique ID of the timer
+         */
+        public long setPeriodic(long initialDelay, long delay, Handler<Long> handler)
+        {
+            return setPeriodic(initialDelay, delay, handler, false);
+        }
+
+        /**
+         * Set a periodic timer to fire every {@code delay} milliseconds with initial delay, at which point
+         * {@code handler} will be called with the id of the timer.
+         *
+         * @param initialDelay the initial delay in milliseconds
+         * @param delay        the delay in milliseconds, after which the timer will fire
+         * @param handler      the handler that will be called with the timer ID when the timer fires
+         * @param ordered      if true then executeBlocking is called several times on the same context, the
+         *                     executions for that context will be executed serially, not in parallel. if false
+         *                     then they will be no ordering guarantees
+         * @return the unique ID of the timer
+         */
+        public long setPeriodic(long initialDelay, long delay, Handler<Long> handler, boolean ordered)
+        {
+            return vertx.setPeriodic(initialDelay,
+                                     delay,
+                                     id -> workerExecutor.executeBlocking(() -> {
+                                         handler.handle(id);
+                                         return id;

Review Comment:
   ok, I think it's a valid concern. I will double check, but it looks like the exception now should be `RejectedExecutionException`. I assume the intention was to maintain the pool API expectations instead of throwing an `IllegalStateException` 



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
common/src/main/java/org/apache/cassandra/sidecar/common/JmxClient.java:
##########
@@ -65,94 +64,34 @@ public class JmxClient implements NotificationListener, Closeable
     private final Supplier<String> roleSupplier;
     private final Supplier<String> passwordSupplier;
     private final BooleanSupplier enableSslSupplier;
-    private final int jmxConnectionMaxRetries;
-    private final long jmxConnectionRetryDelayMillis;
+    private final int connectionMaxRetries;
+    private final long connectionRetryDelayMillis;
 
-    /**
-     * Creates a new client with the provided {@code host} and {@code port}.
-     *
-     * @param host the host of the JMX service
-     * @param port the port of the JMX service
-     */
-    public JmxClient(String host, int port)
-    {
-        this(host, port, null, null, false);
-    }
 
     /**
-     * Creates a new client with the provided parameters
+     * Creates a new JMX client with {@link Builder} options.
      *
-     * @param host      the host of the JMX service
-     * @param port      the port of the JMX service
-     * @param role      the JMX role used for authentication
-     * @param password  the JMX role password used for authentication
-     * @param enableSSl true if SSL is enabled for JMX, false otherwise
+     * @param builder the builder options
      */
-    public JmxClient(String host, int port, String role, String password, boolean enableSSl)
-    {
-        this(buildJmxServiceURL(host, port), () -> role, () -> password, () -> enableSSl);
-    }
-
-    /**
-     * Creates a new client with the provided parameters
-     *
-     * @param host                       the host of the JMX service
-     * @param port                       the port of the JMX service
-     * @param role                       the JMX role used for authentication
-     * @param password                   the JMX role password used for authentication
-     * @param enableSSl                  true if SSL is enabled for JMX, false otherwise
-     * @param connectionMaxRetries       the maximum number of connection retries before failing to connect
-     * @param connectionRetryDelayMillis the number of milliseconds to delay between connection retries
-     */
-    public JmxClient(String host, int port, String role, String password,
-                     boolean enableSSl, int connectionMaxRetries, long connectionRetryDelayMillis)
-    {
-        this(buildJmxServiceURL(host, port), () -> role, () -> password, () -> enableSSl,
-             connectionMaxRetries, connectionRetryDelayMillis);
-    }
-
-    @VisibleForTesting
-    JmxClient(JMXServiceURL jmxServiceURL)
-    {
-        this(jmxServiceURL, () -> null, () -> null, () -> false);
-    }
-
-    @VisibleForTesting
-    JmxClient(JMXServiceURL jmxServiceURL, String role, String password)
-    {
-        this(jmxServiceURL, () -> role, () -> password, () -> false);
-    }
-
-    public JmxClient(String host,
-                     int port,
-                     Supplier<String> roleSupplier,
-                     Supplier<String> passwordSupplier,
-                     BooleanSupplier enableSslSupplier)
+    private JmxClient(Builder builder)

Review Comment:
   decided to go with the builder here, because the number of constructors was getting crazy



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/routes/GossipInfoHandler.java:
##########
@@ -24,11 +24,11 @@
 import io.vertx.core.http.HttpServerRequest;
 import io.vertx.core.net.SocketAddress;
 import io.vertx.ext.web.RoutingContext;
-import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
 import org.apache.cassandra.sidecar.common.ClusterMembershipOperations;
 import org.apache.cassandra.sidecar.common.data.GossipInfoResponse;
 import org.apache.cassandra.sidecar.common.utils.GossipInfoParser;
 import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.utils.CassandraAdapterDelegate;

Review Comment:
   I moved some classes away from common (where they didn't make sense), and put them in server.



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/config/TrafficShapingConfiguration.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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 org.apache.cassandra.sidecar.config;
+
+import io.netty.handler.traffic.AbstractTrafficShapingHandler;
+import io.netty.handler.traffic.GlobalTrafficShapingHandler;
+
+/**
+ * Configuration for the global traffic shaping options. These TCP server options enable configuration of
+ * bandwidth limiting. Both inbound and outbound bandwidth can be limited through these options.
+ */
+public interface TrafficShapingConfiguration
+{
+    /**
+     * Default inbound bandwidth limit in bytes/sec = 0 (0 implies unthrottled)
+     */
+    long DEFAULT_INBOUND_GLOBAL_BANDWIDTH_LIMIT = 0;

Review Comment:
   yeah, I guess these are specific to yaml



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
@@ -20,7 +20,8 @@
 
 import java.util.List;
 
-import org.apache.cassandra.sidecar.common.CassandraAdapterDelegate;
+import org.apache.cassandra.sidecar.utils.CassandraAdapterDelegate;

Review Comment:
   `CassandraAdapterDelegate` does not seem to fit in the `utils` package. In the existing packages, `cluster` would be a better fit. 



##########
src/main/dist/conf/sidecar.yaml:
##########
@@ -68,10 +68,20 @@ sidecar:
   port: 9043
   request_idle_timeout_millis: 300000 # this field expects integer value
   request_timeout_millis: 300000
+  tcp_keep_alive: false
+  accept_backlog: 1024
+  server_verticle_instances: 1
   throttle:
     stream_requests_per_sec: 5000
     delay_sec: 5
     timeout_sec: 10
+  traffic_shaping:
+    inbound_global_bandwidth_bps: 0               # 0 implies unthrottled, the inbound bandwidth in bytes per second
+    outbound_global_bandwidth_bps: 0              # 0 implies unthrottled, the outbound bandwidth in bytes per second
+    peak_outbound_global_bandwidth_bps: 419430400 # 400 Mbps, the peak outbound bandwidth in bytes per second

Review Comment:
   `Mbps` means megabits per seconds. 
   The value here is indeed `MBps`.
   
   It is too easy to be confused with the abbreviation, i.e. upper B and lower b. 
   
   I would just put this for the comment 
   
   "the peak outbound bandwidth in bytes per second. The default is 400 mebibytes per second"



##########
src/main/java/org/apache/cassandra/sidecar/concurrent/ExecutorPools.java:
##########
@@ -119,9 +120,45 @@ public long setPeriodic(long delay, Handler<Long> handler)
         public long setPeriodic(long delay, Handler<Long> handler, boolean ordered)
         {
             return vertx.setPeriodic(delay,
-                                     id -> workerExecutor.executeBlocking(promise -> {
+                                     id -> workerExecutor.executeBlocking(() -> {
                                          handler.handle(id);
-                                         promise.complete();
+                                         return id;
+                                     }, ordered));

Review Comment:
   instead of scheduling. It can just call the other `setPeriodic` overloads.
   
   ```
           public long setPeriodic(long delay, Handler<Long> handler, boolean ordered)
           {
               return setPeriodic(delay, delay, handler, ordered);
           }
   ```



##########
src/main/java/org/apache/cassandra/sidecar/config/TrafficShapingConfiguration.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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 org.apache.cassandra.sidecar.config;
+
+import io.netty.handler.traffic.AbstractTrafficShapingHandler;
+import io.netty.handler.traffic.GlobalTrafficShapingHandler;
+
+/**
+ * Configuration for the global traffic shaping options. These TCP server options enable configuration of
+ * bandwidth limiting. Both inbound and outbound bandwidth can be limited through these options.
+ */
+public interface TrafficShapingConfiguration
+{
+    /**
+     * Default inbound bandwidth limit in bytes/sec = 0 (0 implies unthrottled)
+     */
+    long DEFAULT_INBOUND_GLOBAL_BANDWIDTH_LIMIT = 0;

Review Comment:
   Why defining the constants in the interface? How about moving them to `TrafficShapingConfigurationImpl`?



##########
src/main/java/org/apache/cassandra/sidecar/config/ServiceConfiguration.java:
##########
@@ -33,11 +36,33 @@ public interface ServiceConfiguration
      */
     String host();
 
+    /**
+     * Returns a list of interfaces where the Sidecar process will bind and listen for connections. Defaults to
+     * the configured {@link #host()}.
+     *
+     * @return a list of interfaces where Sidecar will listen
+     */
+    default List<String> listenInterfaces()
+    {
+        return Collections.singletonList(Objects.requireNonNull(host(), "host must be provided"));
+    }
+
     /**
      * @return Sidecar's HTTP REST API port
      */
     int port();
 
+    /**
+     * Returns a list of ports where the Sidecar process will bind and listen for connections. Defaults to
+     * the configured {@link #port()}
+     *
+     * @return a list of ports where Sidecar will listen
+     */
+    default List<Integer> ports()
+    {
+        return Collections.singletonList(port());

Review Comment:
   The matrix of network interface array and port array is confusing. 
   For example, for interface array [i1, i2] and port array [8000, 9000], does it listens to [i1:8000, i1:9000, i2:8000, i2:9000] or [i1:8000, i2:9000]?
   
   For better API clarity, I would just return a list of `SocketAddress`. 



##########
src/main/java/org/apache/cassandra/sidecar/config/SslConfiguration.java:
##########
@@ -28,6 +28,29 @@ public interface SslConfiguration
      */
     boolean enabled();
 
+    /**
+     * Returns {@code true} if the OpenSSL engine should be used, {@code false} otherwise. The OpenSSL engine
+     * will only be enabled if the native libraries for OpenSSL have been loaded correctly.
+     *
+     * @return {@code true} if the OpenSSL engine should be used, {@code false} otherwise

Review Comment:
   nit: 
   - duplicated sentences. 
   - Since configuring `true` does not guarantee OpenSSL is picked, it is only an intent or preference. How about renaming the config to `preferOpenSSL`?



##########
src/main/java/org/apache/cassandra/sidecar/tasks/HealthCheckPeriodicTask.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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 org.apache.cassandra.sidecar.tasks;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import org.apache.cassandra.sidecar.cluster.InstancesConfig;
+import org.apache.cassandra.sidecar.concurrent.ExecutorPools;
+import org.apache.cassandra.sidecar.config.SidecarConfiguration;
+
+/**
+ * Periodically checks the health of every instance configured in the {@link InstancesConfig}.
+ */
+@Singleton
+public class HealthCheckPeriodicTask implements PeriodicTask
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger(HealthCheckPeriodicTask.class);
+    private final SidecarConfiguration configuration;
+    private final InstancesConfig instancesConfig;
+    private final ExecutorPools.TaskExecutorPool internalPool;
+
+    @Inject
+    public HealthCheckPeriodicTask(SidecarConfiguration configuration,
+                                   InstancesConfig instancesConfig,
+                                   ExecutorPools executorPools)
+    {
+        this.configuration = configuration;
+        this.instancesConfig = instancesConfig;
+        internalPool = executorPools.internal();
+    }
+
+    @Override
+    public long initialDelay()
+    {
+        return configuration.healthCheckConfiguration().initialDelayMillis();
+    }
+
+    @Override
+    public long delay()
+    {
+        return configuration.healthCheckConfiguration().checkIntervalMillis();
+    }
+
+    /**
+     * Run health checks on all the configured instances
+     */
+    @Override
+    public void execute()
+    {
+        instancesConfig.instances()
+                       .forEach(instanceMetadata -> internalPool.executeBlocking(promise -> {
+                           try
+                           {
+                               instanceMetadata.delegate().healthCheck();
+                               promise.complete();
+                           }
+                           catch (Throwable cause)
+                           {
+                               promise.fail(cause);
+                               LOGGER.error("Unable to complete health check on instance={}",
+                                            instanceMetadata.id(), cause);
+                           }
+                       }));

Review Comment:
   nit: submit with `unordered == true`.



##########
src/main/java/org/apache/cassandra/sidecar/concurrent/ExecutorPools.java:
##########
@@ -119,9 +120,45 @@ public long setPeriodic(long delay, Handler<Long> handler)
         public long setPeriodic(long delay, Handler<Long> handler, boolean ordered)
         {
             return vertx.setPeriodic(delay,
-                                     id -> workerExecutor.executeBlocking(promise -> {
+                                     id -> workerExecutor.executeBlocking(() -> {
                                          handler.handle(id);
-                                         promise.complete();
+                                         return id;
+                                     }, ordered));
+        }
+
+        /**
+         * Set a periodic timer to fire every {@code delay} milliseconds with initial delay, at which point
+         * {@code handler} will be called with the id of the timer.
+         *
+         * @param initialDelay the initial delay in milliseconds
+         * @param delay        the delay in milliseconds, after which the timer will fire
+         * @param handler      the handler that will be called with the timer ID when the timer fires
+         * @return the unique ID of the timer
+         */
+        public long setPeriodic(long initialDelay, long delay, Handler<Long> handler)
+        {
+            return setPeriodic(initialDelay, delay, handler, false);
+        }
+
+        /**
+         * Set a periodic timer to fire every {@code delay} milliseconds with initial delay, at which point
+         * {@code handler} will be called with the id of the timer.
+         *
+         * @param initialDelay the initial delay in milliseconds
+         * @param delay        the delay in milliseconds, after which the timer will fire
+         * @param handler      the handler that will be called with the timer ID when the timer fires
+         * @param ordered      if true then executeBlocking is called several times on the same context, the
+         *                     executions for that context will be executed serially, not in parallel. if false
+         *                     then they will be no ordering guarantees
+         * @return the unique ID of the timer
+         */
+        public long setPeriodic(long initialDelay, long delay, Handler<Long> handler, boolean ordered)
+        {
+            return vertx.setPeriodic(initialDelay,
+                                     delay,
+                                     id -> workerExecutor.executeBlocking(() -> {
+                                         handler.handle(id);
+                                         return id;

Review Comment:
   Although the new library suggests to use the new API that takes `Callable`, one difference I noticed in the `WorkerExecutorImpl` is that the implementation with `Callable` does not check whether Worker executor is closed, meaning the callable is scheduled to the closing pool regardless. 
   
   It could be a concern.
   
   I would revert and use the deprecated API. 



##########
src/main/java/org/apache/cassandra/sidecar/server/MainModule.java:
##########
@@ -241,11 +237,27 @@ public Router vertxRouter(Vertx vertx,
         return router;
     }
 
+    @Provides
+    @Singleton
+    public PeriodicTaskExecutor periodicTaskExecutor(Vertx vertx,
+                                                     ExecutorPools executorPools,
+                                                     HealthCheckPeriodicTask healthCheckTask)
+    {
+        EventBus eventBus = vertx.eventBus();
+        PeriodicTaskExecutor executor = new PeriodicTaskExecutor(executorPools);
+        eventBus.localConsumer(ON_SERVER_START, message -> executor.schedule(healthCheckTask));
+        eventBus.localConsumer(ON_SERVER_STOP, message -> executor.unschedule(healthCheckTask));

Review Comment:
   nit: the event bus message registration for health check fits better in `HealthCheckPeriodicTask` constructor. 
   
   The method here should just return an executor



##########
src/main/java/org/apache/cassandra/sidecar/server/SidecarServerEvents.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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 org.apache.cassandra.sidecar.server;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import io.vertx.core.Vertx;
+import io.vertx.core.eventbus.EventBus;
+import io.vertx.core.eventbus.MessageConsumer;
+import io.vertx.core.json.JsonArray;
+import io.vertx.core.json.JsonObject;
+import org.apache.cassandra.sidecar.cluster.InstancesConfig;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+
+/**
+ * Defines the {@link io.vertx.core.eventbus.EventBus} addresses where different notifications will be published
+ * during Sidecar startup/shutdown, as well as CQL connection availability.
+ *
+ * <p>The messages can be published multiple times depending on whether Sidecar is started or stopped
+ * during the lifetime of the application. Implementing consumers will need to deal with this expectation
+ * internally.
+ * <p>
+ * The expectation is that:
+ * <ul>
+ * <li>{@link #ON_SERVER_START} will happen first
+ * <li>{@link #ON_SERVER_STOP} can happen before {@link #ON_ALL_CASSANDRA_CQL_READY}
+ * <li>{@link #ON_SERVER_START} can only happen for any subsequent calls only after a {@link #ON_SERVER_STOP} message
+ * <li>{@link #ON_ALL_CASSANDRA_CQL_READY} might never happen
+ * <li>{@link #ON_CASSANDRA_CQL_READY} can be called multiple times with different cassandraInstanceId values
+ * <li>{@link #ON_CASSANDRA_CQL_DISCONNECTED} can be called multiple times with different cassandraInstanceId values
+ * </ul>
+ * <p>
+ * However, implementers should choose to implement methods assuming no guarantees to the event sequence.
+ */
+@Singleton
+public class SidecarServerEvents
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger(SidecarServerEvents.class);
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where server start events will be published. Server start
+     * will be published whenever Sidecar has successfully started and is ready listening for requests.
+     */
+    public static final String ON_SERVER_START = SidecarServerEvents.class.getName() + ".ON_SERVER_START";
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where server stop/shutdown events will be published.
+     * Server stop events will be published whenever Sidecar is stopping or shutting down.
+     */
+    public static final String ON_SERVER_STOP = SidecarServerEvents.class.getName() + ".ON_SERVER_STOP";
+
+    /**
+     * The {@link io.vertx.core.eventbus.EventBus} address where events will be published when a CQL connection for
+     * a given instance has been established. The instance identifier will be passed as part of the message.
+     */
+    public static final String ON_CASSANDRA_CQL_READY = SidecarServerEvents.class.getName() + ".ON_CASSANDRA_CQL_READY";

Review Comment:
   Instead of hardcode the names, you can declare the enum instead. It store all the name in the list that can be accessed using the enum's ordinal. 
   
   ```java
       enum Event
       {
           ON_SERVER_START;
   
           private static List<String> eventNames = Arrays.stream(values())
                                                          .map(e -> e.getDeclaringClass().getName() + '.' + e.name())
                                                          .collect(Collectors.toList());
   
           String eventName()
           {
               return eventNames.get(ordinal());
           }
       }
   ```



-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh closed pull request #72: CASSANDRASC-77 Upgrade to vertx 4.4.6
URL: https://github.com/apache/cassandra-sidecar/pull/72


-- 
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] CASSANDRASC-77 Upgrade to vertx 4.4.6 [cassandra-sidecar]

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


##########
src/main/dist/conf/sidecar.yaml:
##########
@@ -68,10 +68,20 @@ sidecar:
   port: 9043
   request_idle_timeout_millis: 300000 # this field expects integer value
   request_timeout_millis: 300000
+  tcp_keep_alive: false
+  accept_backlog: 1024
+  server_verticle_instances: 1
   throttle:
     stream_requests_per_sec: 5000
     delay_sec: 5
     timeout_sec: 10
+  traffic_shaping:
+    inbound_global_bandwidth_bps: 0               # 0 implies unthrottled, the inbound bandwidth in bytes per second
+    outbound_global_bandwidth_bps: 0              # 0 implies unthrottled, the outbound bandwidth in bytes per second
+    peak_outbound_global_bandwidth_bps: 419430400 # 400 Mbps, the peak outbound bandwidth in bytes per second

Review Comment:
   yeah! thanks



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