pauloricardomg commented on code in PR #277:
URL: https://github.com/apache/cassandra-sidecar/pull/277#discussion_r2594248352


##########
server/src/main/java/org/apache/cassandra/sidecar/lifecycle/ProcessLifecycleProvider.java:
##########
@@ -18,38 +18,232 @@
 
 package org.apache.cassandra.sidecar.lifecycle;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.exceptions.ConfigurationException;
+import org.jetbrains.annotations.VisibleForTesting;
 
 /**
- * A {@link LifecycleProvider} that manages Cassandra instances as OS 
processes.
- * <p>
- * This implementation is a placeholder and is not yet implemented.
+ * Manage the lifecycle of Cassandra instances running on local processes
  */
 public class ProcessLifecycleProvider implements LifecycleProvider
 {
+    static final String OPT_CASSANDRA_HOME = "cassandra_home";
+    static final String OPT_CASSANDRA_CONF_DIR = "cassandra_conf_dir";
+    static final String OPT_CASSANDRA_LOG_DIR = "cassandra_log_dir";
+    static final String OPT_STATE_DIR = "state_dir";
+
+    protected static final Logger LOG = 
LoggerFactory.getLogger(ProcessLifecycleProvider.class);
+    public static final long CASSANDRA_PROCESS_TIMEOUT_MS = 
Long.getLong("cassandra.sidecar.lifecycle.process.timeout.ms", 120_000L);
+
+    private final String lifecycleDir;
+    private final String defaultCassandraHome;
+    private final Map<String, String> defaultJvmProperties = new HashMap<>();
+    private final Map<String, String> defaultEnvVars = new HashMap<>();
+
     public ProcessLifecycleProvider(Map<String, String> params)
     {
-        // Params unused for now
+        // Extract any JVM properties or environment variables from the params
+        for (Map.Entry<String, String> entry : params.entrySet())
+        {
+            if (entry.getKey().startsWith("cassandra."))
+            {
+                defaultJvmProperties.put(entry.getKey(), entry.getValue());
+            }
+            else if (entry.getKey().startsWith("env."))
+            {
+                defaultEnvVars.put(entry.getKey().replaceAll("env.", ""), 
entry.getValue());
+            }
+        }
+        this.lifecycleDir = params.get(OPT_STATE_DIR);
+        this.defaultCassandraHome = params.get(OPT_CASSANDRA_HOME);
+        if (lifecycleDir == null || lifecycleDir.isEmpty())
+        {
+            throw new ConfigurationException("Configuration property '" + 
OPT_STATE_DIR + "' must be set for ProcessLifecycleProvider");
+        }
+        if (defaultCassandraHome == null || defaultCassandraHome.isEmpty())
+        {
+            throw new ConfigurationException("Configuration property '" + 
OPT_CASSANDRA_HOME + "' must be set for ProcessLifecycleProvider");
+        }
     }
 
     @Override
     public void start(InstanceMetadata instance)
     {
-        throw new UnsupportedOperationException("Not implemented yet");
+        if (isCassandraProcessRunning(instance))
+        {
+            LOG.info("Cassandra instance {} is already running.", 
instance.host());
+            return;
+        }
+        startCassandra(instance);
     }
 
     @Override
     public void stop(InstanceMetadata instance)
     {
-        throw new UnsupportedOperationException("Not implemented yet");
-
+        if (!isCassandraProcessRunning(instance))
+        {
+            LOG.info("Cassandra instance {} is already stopped.", 
instance.host());
+            return;
+        }
+        stopCassandra(instance);
     }
 
     @Override
     public boolean isRunning(InstanceMetadata instance)
     {
-        throw new UnsupportedOperationException("Not implemented yet");
+        return isCassandraProcessRunning(instance);
+    }
+
+    private void startCassandra(InstanceMetadata instance)
+    {
+        ProcessRuntimeConfiguration runtimeConfig = 
getRuntimeConfiguration(instance);
+        try
+        {
+            String stdoutLocation = 
getStdoutLocation(runtimeConfig.instanceName());
+            String stderrLocation = 
getStderrLocation(runtimeConfig.instanceName());
+            String pidFileLocation = 
getPidFileLocation(runtimeConfig.instanceName());
+            ProcessBuilder processBuilder = 
runtimeConfig.buildStartCommand(pidFileLocation,
+                                                                            
stdoutLocation,
+                                                                            
stderrLocation);
+            LOG.info("Starting Cassandra instance {} with command: {}", 
runtimeConfig.instanceName(), processBuilder.command());
+
+            Process process = processBuilder.start();
+            process.waitFor(CASSANDRA_PROCESS_TIMEOUT_MS, 
TimeUnit.MILLISECONDS); // blocking call, make async?

Review Comment:
   My understanding is that `vertx.setPeriodic` will return immediatelly right 
? In which case, `LifecycleProvider.start` could return before the Cassandra 
process is initialized.
   
   The issue is that 
[LifecycleManager.submitStartTask](https://github.com/pauloricardomg/cassandra-sidecar/blob/CASSSIDECAR-340/server/src/main/java/org/apache/cassandra/sidecar/lifecycle/LifecycleManager.java#L146-L147)
 assumes `LifecycleProvider.start` is synchronous and will only return after 
the process is guaranteed to be started. Are you suggesting making 
`LifecycleProvider.start` asynchronous and return a `Future` instead? Since 
this is a departure from the current synchronous design and may require many 
changes I would prefer to do this in a follow-up ticket if that works for you.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to