yifan-c commented on code in PR #249: URL: https://github.com/apache/cassandra-sidecar/pull/249#discussion_r2301707118
########## client-common/src/main/java/org/apache/cassandra/sidecar/common/response/data/ActiveCompactionEntry.java: ########## @@ -0,0 +1,296 @@ +/* + * 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.List; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.common.DataObjectBuilder; + +/** + * Represents an active compaction entry + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class ActiveCompactionEntry +{ + public static final String ID = "id"; + public static final String KEYSPACE = "keyspace"; + public static final String TABLE = "table"; + public static final String TASK_TYPE = "taskType"; + public static final String COMPLETED_BYTES = "completedBytes"; + public static final String TOTAL_BYTES = "totalBytes"; + public static final String PERCENT_COMPLETED = "percentCompleted"; + public static final String SS_TABLES = "ssTables"; + public static final String TARGET_DIRECTORY = "targetDirectory"; + + private final String id; + private final String keyspace; + private final String table; + private final String taskType; + private final long completedBytes; + private final long totalBytes; + private final double percentCompleted; + private final List<String> ssTables; + private final String targetDirectory; + + private ActiveCompactionEntry(Builder builder) + { + this.id = builder.id; + this.keyspace = builder.keyspace; + this.table = builder.table; + this.taskType = builder.taskType; + this.completedBytes = builder.completedBytes; + this.totalBytes = builder.totalBytes; + this.percentCompleted = builder.percentCompleted; + this.ssTables = builder.ssTables; + this.targetDirectory = builder.targetDirectory; + } + + /** + * Constructs a new {@link ActiveCompactionEntry}. + * + * @param id compaction ID + * @param keyspace keyspace name + * @param table table name + * @param taskType type of compaction task + * @param completedBytes completed compaction in bytes + * @param totalBytes total compaction in bytes + * @param percentCompleted percentage of completed compactions + * @param ssTables list of SSTables being compacted + * @param targetDirectory target directory for output + */ + @JsonCreator + public ActiveCompactionEntry(@JsonProperty(ID) String id, + @JsonProperty(KEYSPACE) String keyspace, + @JsonProperty(TABLE) String table, + @JsonProperty(TASK_TYPE) String taskType, + @JsonProperty(COMPLETED_BYTES) long completedBytes, + @JsonProperty(TOTAL_BYTES) long totalBytes, + @JsonProperty(PERCENT_COMPLETED) double percentCompleted, + @JsonProperty(SS_TABLES) List<String> ssTables, + @JsonProperty(TARGET_DIRECTORY) String targetDirectory) + { + this.id = id; + this.keyspace = keyspace; + this.table = table; + this.taskType = taskType; + this.completedBytes = completedBytes; + this.totalBytes = totalBytes; + this.percentCompleted = percentCompleted; + this.ssTables = ssTables; + this.targetDirectory = targetDirectory; + } + + @JsonProperty(ID) + public String id() + { + return id; + } + + @JsonProperty(KEYSPACE) + public String keyspace() + { + return keyspace; + } + + @JsonProperty(TABLE) + public String table() + { + return table; + } + + @JsonProperty(TASK_TYPE) + public String taskType() + { + return taskType; + } + + @JsonProperty(COMPLETED_BYTES) + public long completedBytes() + { + return completedBytes; + } + + @JsonProperty(TOTAL_BYTES) + public long totalBytes() + { + return totalBytes; + } + + @JsonProperty(PERCENT_COMPLETED) + public double percentCompleted() + { + return percentCompleted; + } + + @JsonProperty(SS_TABLES) + public List<String> ssTables() + { + return ssTables; + } + + @JsonProperty(TARGET_DIRECTORY) + public String targetDirectory() + { + return targetDirectory; + } + + public static Builder builder() + { + return new Builder(); + } + + /** + * {@code ActiveCompactionEntry} builder static inner class. + */ + public static final class Builder implements DataObjectBuilder<Builder, ActiveCompactionEntry> + { + private String id; + private String keyspace; + private String table; + private String taskType; + private long completedBytes; + private long totalBytes; + private double percentCompleted; + private List<String> ssTables; + private String targetDirectory; + + private Builder() + { + } + + @Override + public Builder self() + { + return this; + } + + /** + * Sets the {@code id} and returns a reference to this Builder enabling method chaining. + * + * @param id the {@code id} to set + * @return a reference to this Builder + */ + public Builder id(String id) + { + return update(b -> b.id = id); + } + + /** + * Sets the {@code keyspace} and returns a reference to this Builder enabling method chaining. + * + * @param keyspace the {@code keyspace} to set + * @return a reference to this Builder + */ + public Builder keyspace(String keyspace) + { + return update(b -> b.keyspace = keyspace); + } + + /** + * Sets the {@code table} and returns a reference to this Builder enabling method chaining. + * + * @param table the {@code table} to set + * @return a reference to this Builder + */ + public Builder table(String table) + { + return update(b -> b.table = table); + } + + /** + * Sets the {@code taskType} and returns a reference to this Builder enabling method chaining. + * + * @param taskType the {@code taskType} to set + * @return a reference to this Builder + */ + public Builder taskType(String taskType) + { + return update(b -> b.taskType = taskType); + } + + /** + * Sets the {@code completedBytes} and returns a reference to this Builder enabling method chaining. + * + * @param completedBytes the {@code completedBytes} to set + * @return a reference to this Builder + */ + public Builder completedBytes(long completedBytes) + { + return update(b -> b.completedBytes = completedBytes); + } + + /** + * Sets the {@code totalBytes} and returns a reference to this Builder enabling method chaining. + * + * @param totalBytes the {@code totalBytes} to set + * @return a reference to this Builder + */ + public Builder totalBytes(long totalBytes) + { + return update(b -> b.totalBytes = totalBytes); + } + + /** + * Sets the {@code percentCompleted} and returns a reference to this Builder enabling method chaining. + * + * @param percentCompleted the {@code percentCompleted} to set + * @return a reference to this Builder + */ + public Builder percentCompleted(double percentCompleted) + { + return update(b -> b.percentCompleted = percentCompleted); + } + + /** + * Sets the {@code ssTables} and returns a reference to this Builder enabling method chaining. + * + * @param ssTables the {@code ssTables} to set + * @return a reference to this Builder + */ + public Builder ssTables(List<String> ssTables) Review Comment: `sstables` ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/response/data/ActiveCompactionEntry.java: ########## @@ -0,0 +1,296 @@ +/* + * 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.List; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.common.DataObjectBuilder; + +/** + * Represents an active compaction entry + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class ActiveCompactionEntry +{ + public static final String ID = "id"; + public static final String KEYSPACE = "keyspace"; + public static final String TABLE = "table"; + public static final String TASK_TYPE = "taskType"; + public static final String COMPLETED_BYTES = "completedBytes"; + public static final String TOTAL_BYTES = "totalBytes"; + public static final String PERCENT_COMPLETED = "percentCompleted"; + public static final String SS_TABLES = "ssTables"; Review Comment: It is less common to break up the term `sstable` ```suggestion public static final String SSTABLES = "sstables"; ``` ########## integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/CassandraStatsIntegrationTest.java: ########## @@ -256,4 +280,284 @@ void assertClientStatsResponse(HttpResponse<Buffer> response, Map<String, Boolea } } } + + @Test + void testCompactionStatsRetrieval() + { + logger.info("Starting compaction stats test with {} tables", COMPACTION_TEST_TABLES.size()); + + // Generate SSTables for all test tables + for (QualifiedName tableName : COMPACTION_TEST_TABLES) + { + generateSSTables(tableName, 100); + } + + // Create threads to trigger compaction on all tables + List<Thread> compactionThreads = new ArrayList<>(); + for (QualifiedName tableName : COMPACTION_TEST_TABLES) + { + Thread thread = new Thread(() -> triggerCompactionForTable(tableName)); + compactionThreads.add(thread); + } + + // Start all compaction threads + for (Thread thread : compactionThreads) + { + thread.start(); + } + + // Poll immediately and repeatedly to catch active compactions + CompactionStatsResponse stats = null; + HttpResponse<Buffer> response; + boolean foundActiveCompactions; + + for (int attempt = 0; attempt < MAX_POLL_ATTEMPTS; attempt++) + { + try + { + response = getBlocking( + trustedClient().get(serverWrapper.serverPort, "localhost", COMPACTION_STATS_ROUTE) + .send() + .expecting(HttpResponseExpectation.SC_OK)); + + stats = response.bodyAsJson(CompactionStatsResponse.class); + foundActiveCompactions = !stats.activeCompactions().isEmpty(); + + if (foundActiveCompactions) + { + logger.info("SUCCESS: Found {} active compactions on attempt {}", + stats.activeCompactionsCount(), attempt + 1); + break; + } + else + { + logger.info("Attempt {}: No active compactions yet", attempt + 1); + } + + Thread.sleep(100); // Short sleep between attempts + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + break; + } + } + + // Wait for all compaction threads to complete + for (Thread thread : compactionThreads) + { + try + { + thread.join(5000); + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + break; + } + } + assertThat(stats).isNotNull(); + logger.info("Response:{}", stats); + validateCompactionStatsResponse(stats); + } + + + private void generateSSTables(QualifiedName tableName, int numSSTables) + { + for (int batch = 0; batch < numSSTables; batch++) + { + for (int i = batch * 1000; i < (batch + 1) * 1000; i++) + { + String statement = String.format("INSERT INTO %s (id, data) VALUES (%d, '%s');", + tableName, i, "data" + i); + cluster.schemaChangeIgnoringStoppedInstances(statement); + } + cluster.stream().forEach(instance -> instance.flush(TEST_KEYSPACE)); + } + } + + private void triggerCompactionForTable(QualifiedName tableName) + { + cluster.stream().forEach(instance -> + { + try + { + instance.nodetool("compact", tableName.keyspace(), tableName.table()); + } + catch (Exception e) + { + logger.warn("Failed to trigger compaction for {}: {}", tableName, e.getMessage()); + } + }); + } + + private void validateCompactionStatsResponse(CompactionStatsResponse stats) + { + assertThat(stats).isNotNull(); + + // Basic counters validation + assertThat(stats.concurrentCompactors()).isGreaterThanOrEqualTo(0); + assertThat(stats.totalPendingTasks()).isGreaterThanOrEqualTo(0); + assertThat(stats.completedCompactions()).isGreaterThanOrEqualTo(0); + assertThat(stats.dataCompacted()).isGreaterThanOrEqualTo(0); + assertThat(stats.abortedCompactions()).isGreaterThanOrEqualTo(0); + assertThat(stats.reducedCompactions()).isGreaterThanOrEqualTo(0); + assertThat(stats.sstablesDroppedFromCompaction()).isGreaterThanOrEqualTo(0); + + // Pending tasks validation + assertThat(stats.pendingTasks()).isNotNull(); + + // Validate each pending task entry if there are any + if (!stats.pendingTasks().isEmpty()) + { + validatePendingTasks(stats); + } + + // Completion rates validation + assertThat(stats.completedCompactionsRate()).isNotNull(); + + // Validate mean rate format is X.XX/hour + assertThat(stats.completedCompactionsRate().meanRate()) + .as("Mean rate should not be null") + .isNotNull(); + + // Validate fifteen minute rate format is X.XX/minute + assertThat(stats.completedCompactionsRate().fifteenMinuteRate()) + .as("Fifteen minute rate should not be null") + .isNotNull(); + + // Active compactions validation + assertThat(stats.activeCompactions()).isNotNull(); + assertThat(stats.activeCompactionsCount()).isEqualTo(stats.activeCompactions().size()); + assertThat(stats.activeCompactionsRemainingTime()).isGreaterThanOrEqualTo(0L); + + // Detailed active compaction validation when compactions are found + if (!stats.activeCompactions().isEmpty()) + { + validateActiveCompactions(stats); + + logger.info("All {} active compactions validated successfully", stats.activeCompactionsCount()); + } + else + { + logger.info("No active compactions to validate - basic structure validation completed"); + } + + logger.info("Compaction stats validation successful. Active: {}, Completed: {}, Pending: {}", + stats.activeCompactionsCount(), stats.completedCompactions(), stats.totalPendingTasks()); + } + + private void validatePendingTasks(CompactionStatsResponse stats) + { + stats.pendingTasks().forEach((keyspace, tableMap) -> { + assertThat(keyspace) + .as("Pending task keyspace should not be blank") + .isNotBlank(); + assertThat(tableMap) + .as("Pending task table map should not be null") + .isNotNull(); + + tableMap.forEach((table, count) -> { + assertThat(table) + .as("Pending task table name should not be blank") + .isNotBlank(); + assertThat(count) + .as("Pending task count should be non-negative") + .isGreaterThanOrEqualTo(0); + }); + }); + logger.info("Validated {} pending task keyspaces", stats.pendingTasks().size()); + } + + private void validateActiveCompactions(CompactionStatsResponse stats) + { + logger.info("Validating {} active compaction entries", stats.activeCompactionsCount()); + + for (int i = 0; i < stats.activeCompactions().size(); i++) + { + ActiveCompactionEntry compaction = stats.activeCompactions().get(i); + logger.info("Validating active compaction {}: {}", i + 1, compaction.id()); + + // Required fields validation + assertThat(compaction.id()) + .as("Active compaction ID should not be null") + .isNotNull(); + + assertThat(compaction.keyspace()) + .as("Active compaction keyspace should not be null") + .isNotNull() + .isNotBlank(); Review Comment: `isNotBlank` already covers `isNotNull`. Please keep only the `isNotBlank` ########## client-common/src/main/java/org/apache/cassandra/sidecar/common/response/data/ActiveCompactionEntry.java: ########## @@ -0,0 +1,296 @@ +/* + * 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.List; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import org.apache.cassandra.sidecar.common.DataObjectBuilder; + +/** + * Represents an active compaction entry + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class ActiveCompactionEntry Review Comment: It would be nice to name it the same name, `CompactionInfo`, as in Cassandra. Easier to correlate. -- 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-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
