yifan-c commented on code in PR #163:
URL: https://github.com/apache/cassandra-sidecar/pull/163#discussion_r1936764719
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadata.java:
##########
@@ -48,24 +49,48 @@ public interface InstanceMetadata
/**
* @return a list of data directories of cassandra instance
*/
- List<String> dataDirs();
+ @NotNull List<String> dataDirs();
/**
* @return a staging directory of the cassandra instance
*/
String stagingDir();
- /**
- * @return cdc directory of the cassandra instance
- */
- String cdcDir();
-
/**
* @return a {@link CassandraAdapterDelegate} specific for the instance,
or throws when the delegate is unavailable
* @throws CassandraUnavailableException when the Cassandra service is
unavailable
*/
@NotNull CassandraAdapterDelegate delegate() throws
CassandraUnavailableException;
+ /**
+ * @return CDC directory of the cassandra instance, it can be null when
CDC is not enabled for the Cassandra
+ * instance
+ */
+ @Nullable String cdcDir();
Review Comment:
nit: I am a bit concerned about the javadoc. It could mislead the reader
that this method can be used to check whether cdc is enabled
##########
server/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java:
##########
@@ -56,13 +69,19 @@ protected InstanceMetadataImpl(Builder builder)
id = builder.id;
host = builder.host;
port = builder.port;
- dataDirs = builder.dataDirs.stream()
- .map(FileUtils::maybeResolveHomeDirectory)
-
.collect(Collectors.collectingAndThen(Collectors.toList(),
Collections::unmodifiableList));
- stagingDir = FileUtils.maybeResolveHomeDirectory(builder.stagingDir);
- cdcDir = FileUtils.maybeResolveHomeDirectory(builder.cdcDir);
delegate = builder.delegate;
metrics = builder.metrics;
+
+ // Sidecar-managed directories
Review Comment:
👍 to the comments
##########
conf/sidecar.yaml:
##########
@@ -23,6 +23,10 @@ cassandra_instances:
- id: 1
host: localhost1
port: 9042
+ # The instance's storage directory as defined per the cassandra.storagedir
property
+ # which defaults to the $CASSANDRA_HOME/data directory, but can be
configured to any
+ # directory.
Review Comment:
How about also mentioning the evaluation order? The configured `storage_dir`
will be ignored, if the concrete directories of different kinds are configured.
For example, `data_dirs` if configured, the configured value takes the
precedence when evaluating the data directories list.
--
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]
