Re: [PR] CASSSIDECAR-179: Added additional dirs required for live migration [cassandra-sidecar]

["frankgh (via GitHub)" <gi...@apache.org>]

frankgh commented on code in PR #163:
URL: https://github.com/apache/cassandra-sidecar/pull/163#discussion_r1901279571


##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
@@ -66,6 +72,26 @@ public interface InstanceMetadata
      */
     @NotNull CassandraAdapterDelegate delegate() throws CassandraUnavailableException;
 
+    /**
+     * @return commitlog directory of the cassandra instance
+     */
+    @Nullable String commitlogDir();

Review Comment:
   I feel these should be not-null, but we can default to the Cassandra's defaults:
   ```
       /**
        * Returns the commit log directory of the Cassandra instance. If not set, the default directory is
        * {@link #cassandraHomeDir()}/data/commitlog
        *
        * @return commitlog directory of the cassandra instance
        */
       @NotNull
       default String commitlogDir()
       {
           return String.format("%s/data/commitlog", cassandraHomeDir());
       }
   ```



##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -52,15 +65,42 @@ protected InstanceMetadataImpl(Builder builder)
         id = builder.id;
         host = builder.host;
         port = builder.port;
+        cassandraHomeDir = FileUtils.maybeResolveHomeDirectory(builder.cassandraHomeDir);
+        Path cassandraHomeDirPath = cassandraHomeDir == null ?  null : Paths.get(cassandraHomeDir);

Review Comment:
   I think we should disallow the cassandra home dir to be null. 



##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
@@ -66,6 +72,26 @@ public interface InstanceMetadata
      */
     @NotNull CassandraAdapterDelegate delegate() throws CassandraUnavailableException;
 
+    /**
+     * @return commitlog directory of the cassandra instance
+     */
+    @Nullable String commitlogDir();
+
+    /**
+     * @return hints directory of the cassandra instance
+     */
+    @Nullable String hintsDir();
+
+    /**
+     * @return saved caches directory of the cassandra instance
+     */
+    @Nullable String savedCachesDir();
+
+    /**
+     * @return local system data file directory of the cassandra instance
+     */
+    @Nullable String localSystemDataFileDir();

Review Comment:
   I don't really know what this maps to in the cassandra.yaml configuration. Did I miss something?



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