frankgh commented on code in PR #249: URL: https://github.com/apache/cassandra-sidecar/pull/249#discussion_r2308535204
########## adapters/adapters-base/src/test/java/org/apache/cassandra/sidecar/adapters/base/CassandraMetricsOperationsTest.java: ########## Review Comment: can you reformat this file with Sidecar's code style? ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/response/data/PendingCompactionTasks.java: ########## @@ -0,0 +1,61 @@ +/* + * 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.common.response.data; + +import java.util.Map; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +/** + * Represents pending compaction tasks by keyspace and table + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class PendingCompactionTasks Review Comment: is this class used at all? ########## adapters/adapters-base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraMetricsOperations.java: ########## @@ -250,4 +254,30 @@ private List<ClientConnectionEntry> statsToEntries(Stream<ConnectedClientStats> stat.authenticationMode, stat.authenticationMetadata); } + + /** + * {@inheritDoc} + */ + @Override + public Object getCompactionMetric(CompactionStatsMetrics metric) + { + String metricObjectType = String.format(METRICS_OBJ_TYPE_COMPACTION, metric.metricName()); + return queryMetric(metricObjectType, metric.type); + } + + /** + * {@inheritDoc} + */ + @Override + public CompletedCompactionsRateData getCompletedCompactionsRate() + { + // Get rates from meter metric for TotalCompactionsCompleted + String metricObjectType = String.format(METRICS_OBJ_TYPE_COMPACTION, "TotalCompactionsCompleted"); + MeterMetricsJmxOperations metricsProxy = jmxClient.proxy(MeterMetricsJmxOperations.class, metricObjectType); + + return CompletedCompactionsRateData.builder() + .meanRate(metricsProxy.getMeanRate()) Review Comment: formatting is off here ########## adapters/adapters-cassandra41/src/main/java/org/apache/cassandra/sidecar/adapters/cassandra41/Cassandra41StorageOperations.java: ########## @@ -68,4 +71,14 @@ public void takeSnapshot(@NotNull String tag, { super.takeSnapshotInternal(tag, keyspace, table, options); } + + /** + * {@inheritDoc} + */ + @Override + public long getCompactionThroughputBytesPerSec() + { + return jmxClient.proxy(StorageJmxOperations.class, STORAGE_SERVICE_OBJ_NAME) + .getCompactionThroughtputBytesPerSec(); Review Comment: formatting is off ########## adapters/adapters-base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraMetricsOperations.java: ########## @@ -61,6 +66,29 @@ public class CassandraMetricsOperations implements MetricsOperations private final ConnectedClientStatsDatabaseAccessor dbAccessor; protected final JmxClient jmxClient; + private static final String METRICS_OBJ_TYPE_KEYSPACE_TABLE_FORMAT = "org.apache.cassandra.metrics:type=Table,keyspace=%s,scope=%s,name=%s"; + private static final String METRICS_OBJ_TYPE_COMPACTION = "org.apache.cassandra.metrics:type=Compaction,name=%s"; + + // Constants for compaction info map keys + // Unique identifier for the compaction session + public static final String ID = "id"; + // Keyspace name being compacted + public static final String KEYSPACE = "keyspace"; + // Column family (table) name being compacted + public static final String COLUMNFAMILY = "columnfamily"; + // Number of bytes already processed in the compaction + public static final String COMPLETED = "completed"; + // Total number of bytes to be processed in the compaction + public static final String TOTAL = "total"; + // Type of compaction task (e.g., COMPACTION, VALIDATION, etc.) + public static final String TASK_TYPE = "taskType"; + // Unique compaction identifier + public static final String COMPACTION_ID = "compactionId"; + // Comma-separated list of SSTable names involved in the compaction + public static final String SSTABLES = "sstables"; + // Directory where compaction output will be written + public static final String TARGET_DIRECTORY = "targetDirectory"; Review Comment: `ID` seems unused, also let's move these to `org.apache.cassandra.sidecar.service.CompactionStatsService` ########## adapters/adapters-base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraStorageOperations.java: ########## @@ -294,4 +294,24 @@ public void startGossiping() jmxClient.proxy(StorageJmxOperations.class, STORAGE_SERVICE_OBJ_NAME) .startGossiping(); } + + /** + * {@inheritDoc} + */ + @Override + public int getConcurrentCompactors() + { + return jmxClient.proxy(StorageJmxOperations.class, STORAGE_SERVICE_OBJ_NAME) + .getConcurrentCompactors(); + } + + /** + * {@inheritDoc} + */ + @Override + public long getCompactionThroughputBytesPerSec() + { + LOGGER.warn("getCompactionThroughputBytesPerSec is not supported in Cassandra 4.0"); Review Comment: it looks like in 4.0 we have `getCompactionThroughputMbPerSec` . Should we use this and return the value converted to bytes? ########## server/src/main/java/org/apache/cassandra/sidecar/service/CompactionStatsService.java: ########## @@ -0,0 +1,215 @@ +/* + * 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.service; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import org.apache.cassandra.sidecar.common.server.CompactionManagerOperations; +import org.apache.cassandra.sidecar.common.server.MetricsOperations; +import org.apache.cassandra.sidecar.common.server.StorageOperations; +import org.apache.cassandra.sidecar.common.server.data.ActiveCompactionEntryData; +import org.apache.cassandra.sidecar.common.server.data.CompactionStatsData; +import org.apache.cassandra.sidecar.common.server.data.CompactionStatsMetrics; +import org.apache.cassandra.sidecar.common.server.data.CompletedCompactionsRateData; +import org.apache.cassandra.sidecar.common.server.utils.DataTypeUtils; + +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.COLUMNFAMILY; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.COMPACTION_ID; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.COMPLETED; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.KEYSPACE; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.SSTABLES; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.TARGET_DIRECTORY; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.TASK_TYPE; +import static org.apache.cassandra.sidecar.adapters.base.CassandraMetricsOperations.TOTAL; +import static org.apache.cassandra.sidecar.common.server.utils.DataTypeUtils.safeCast; +import static org.apache.cassandra.sidecar.common.server.utils.DataTypeUtils.safeParseLong; + +/** + * Service responsible for fetching compaction stats from various sources including + * CassandraStorageOperations, CassandraMetricsOperations, and CompactionManagerOperations. + */ +public class CompactionStatsService Review Comment: this seems misplaced. This should really live alongside the adapter, because the details are Cassandra-version specific -- 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]

