yifan-c commented on code in PR #197:
URL: https://github.com/apache/cassandra-sidecar/pull/197#discussion_r1962786032
##########
adapters/base/src/main/java/org/apache/cassandra/sidecar/adapters/base/CassandraMetricsOperations.java:
##########
@@ -67,6 +71,87 @@ public CassandraMetricsOperations(JmxClient jmxClient,
CQLSessionProvider sessio
this.dbAccessor = new ConnectedClientStatsDatabaseAccessor(session,
new ConnectedClientsSchema());
}
+ /**
+ * Represents the types of metrics that are queried
+ */
+ public enum MetricType
+ {
+ GAUGE,
+ COUNTER
+ }
+
+ /**
+ * Represents the metrics related to table stats that are supported by the
Sidecar
+ */
+ public enum TableStatsMetrics
+ {
+ SSTABLE_COUNT("LiveSSTableCount", MetricType.GAUGE),
+ DISKSPACE_USED("LiveDiskSpaceUsed", MetricType.COUNTER),
+ TOTAL_DISKSPACE_USED("TotalDiskSpaceUsed", MetricType.COUNTER),
+ SNAPSHOTS_SIZE("SnapshotsSize", MetricType.GAUGE);
+
+ private final String metricName;
+ private final MetricType type;
+
+ TableStatsMetrics(String metricName, MetricType type)
+ {
+ this.metricName = metricName;
+ this.type = type;
+ }
+
+ String metricName()
+ {
+ return metricName;
+ }
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public TableStatsResponse tableStats(QualifiedTableName tableName)
+ {
+ long sstableCount = queryMetric(tableName,
TableStatsMetrics.SSTABLE_COUNT);
+ long diskSpaceUsed = queryMetric(tableName,
TableStatsMetrics.DISKSPACE_USED);
+ long totalDiskSpaceUsed = queryMetric(tableName,
TableStatsMetrics.TOTAL_DISKSPACE_USED);
+ long snapshotsSize = queryMetric(tableName,
TableStatsMetrics.SNAPSHOTS_SIZE);
+
+ return new TableStatsResponse(tableName.keyspace(),
tableName.tableName(), sstableCount, diskSpaceUsed, totalDiskSpaceUsed,
snapshotsSize);
+ }
+
+ private long queryMetric(QualifiedTableName tableName, TableStatsMetrics
metric)
+ {
+ String metricObjectType =
String.format(METRICS_OBJ_TYPE_KEYSPACE_TABLE_FORMAT, tableName.keyspace(),
tableName.tableName(), metric.metricName());
+ MetricsJmxOperations queryResult =
jmxClient.proxy(MetricsJmxOperations.class, metricObjectType);
+ return extractValue(metric, queryResult);
+ }
+
+ private long extractValue(TableStatsMetrics metric, MetricsJmxOperations
queryResult)
+ {
+ switch(metric.type)
+ {
+ case GAUGE: return getValueAsLong(queryResult.getValue());
+ case COUNTER: return queryResult.getCount();
+ default:
+ throw new IllegalArgumentException("Unknown MetricType: " +
metric.type);
+ }
+ }
+
+ private long getValueAsLong(Object value)
+ {
+ if (value instanceof Integer)
+ {
+ return ((Integer) value).longValue();
+ }
+ else if (value instanceof Long)
+ {
+ return (Long) value;
+ }
+ else
+ {
+ throw new IllegalArgumentException("Unsupported value type: " +
value.getClass());
+ }
+ }
Review Comment:
insert an empty line after this
##########
client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java:
##########
@@ -1554,6 +1556,41 @@ public void testConnectedClientStats() throws Exception
}
}
+ @Test
+ public void testTableStats() throws Exception
+ {
+ String testKeyspace = "testKeyspace";
+ String testTable = "testTable";
+ int expectedSstables = 10;
+ long expectedSize = 1024;
+ long expectedTotalSize = 2048;
+ long expectedSnapshotSize = 100;
+
+ TableStatsResponse tableStatsResponse = new
TableStatsResponse(testKeyspace, testTable, expectedSstables, expectedSize,
expectedTotalSize, expectedSnapshotSize);
+ ObjectMapper mapper = new ObjectMapper();
+ MockResponse response = new MockResponse().setResponseCode(OK.code())
+
.setBody(mapper.writeValueAsString(tableStatsResponse));
+ enqueue(response);
+
+ for (MockWebServer server : servers)
+ {
+ SidecarInstanceImpl sidecarInstance =
RequestExecutorTest.newSidecarInstance(server);
+ TableStatsResponse result = client.tableStats(sidecarInstance,
testKeyspace, testTable).get();
+
+ assertThat(result).isNotNull();
+ assertThat(result.sstableCount()).isEqualTo(expectedSstables);
+ assertThat(result.diskSpaceUsedBytes()).isEqualTo(expectedSize);
+
assertThat(result.totalDiskSpaceUsedBytes()).isEqualTo(expectedTotalSize);
+
assertThat(result.snapshotsSizeBytes()).isEqualTo(expectedSnapshotSize);
+ validateResponseServed(server,
+ ApiEndpointsV1.TABLE_STATS_ROUTE
+ .replaceAll(KEYSPACE_PATH_PARAM,
testKeyspace)
+ .replaceAll(TABLE_PATH_PARAM, testTable),
+ req -> { });
Review Comment:
nit: the codestyle should permit empty lambda. The space is unnecessary and
can be removed.
##########
client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java:
##########
@@ -1512,6 +1513,7 @@ void
testCreateRestoreJobShouldNotRetryOnDifferentHostWithBadRequest() throws Ex
@Test
public void testConnectedClientStats() throws Exception
{
+
Review Comment:
revert this new line change
##########
server/src/test/integration/org/apache/cassandra/sidecar/routes/TableStatsHandlerIntegrationTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * 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.routes;
+
+import com.google.common.collect.ImmutableMap;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+import com.datastax.driver.core.Session;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.buffer.Buffer;
+import io.vertx.ext.web.client.HttpResponse;
+import io.vertx.junit5.VertxExtension;
+import org.apache.cassandra.sidecar.common.response.TableStatsResponse;
+import org.apache.cassandra.sidecar.common.server.data.QualifiedTableName;
+import org.apache.cassandra.sidecar.testing.IntegrationTestBase;
+import org.apache.cassandra.testing.CassandraIntegrationTest;
+import org.apache.cassandra.testing.CassandraTestContext;
+
+import static org.apache.cassandra.testing.utils.AssertionUtils.getBlocking;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test the table-stats endpoint with cassandra container.
+ */
+@ExtendWith(VertxExtension.class)
+public class TableStatsHandlerIntegrationTest extends IntegrationTestBase
+{
+ @CassandraIntegrationTest(numDataDirsPerInstance = 4, nodesPerDc = 2,
network = true)
Review Comment:
Is it possible to have single node cluster? I think it should work fine with
table stats.
Another idea is to merge the test with the recently migrated
`ConnectedClientStatsHandlerIntegrationTest` to share the same cluster. Of
course, rename the test class name once merged, say
`CassandraStatsIntegrationTest`.
##########
server-common/src/main/java/org/apache/cassandra/sidecar/common/server/MetricsOperations.java:
##########
@@ -38,4 +40,11 @@ public interface MetricsOperations
* @return the requested stream progress stats
*/
StreamsProgressStats streamsProgressStats();
+ /**
Review Comment:
insert new line
##########
server/src/test/java/org/apache/cassandra/sidecar/routes/TableStatsHandlerTest.java:
##########
@@ -0,0 +1,204 @@
+/*
+ * 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.routes;
+
+import java.util.Collections;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.datastax.driver.core.KeyspaceMetadata;
+import com.datastax.driver.core.Metadata;
+import com.datastax.driver.core.TableMetadata;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import com.google.inject.util.Modules;
+import io.vertx.core.Vertx;
+import io.vertx.ext.web.client.WebClient;
+import io.vertx.ext.web.client.predicate.ResponsePredicate;
+import io.vertx.junit5.VertxExtension;
+import io.vertx.junit5.VertxTestContext;
+import org.apache.cassandra.sidecar.TestModule;
+import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate;
+import org.apache.cassandra.sidecar.cluster.InstancesMetadata;
+import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.apache.cassandra.sidecar.common.response.TableStatsResponse;
+import org.apache.cassandra.sidecar.common.server.MetricsOperations;
+import org.apache.cassandra.sidecar.server.MainModule;
+import org.apache.cassandra.sidecar.server.Server;
+
+import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;
+import static io.netty.handler.codec.http.HttpResponseStatus.OK;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Tests for the {@link TableStatsHandler} class
+ */
+@ExtendWith(VertxExtension.class)
+public class TableStatsHandlerTest
+{
+
+ private static final int EXPECTED_SSTABLES = 10;
+ private static final long EXPECTED_SIZE = 1024;
+ private static final long EXPECTED_TOTAL_SIZE = 2048;
+ private static final long EXPECTED_SNAPSHOT_SIZE = 100;
+ private static final String KEYSPACE = "testkeyspace";
+ private static final String TABLE = "testtable";
+ static final Logger LOGGER =
LoggerFactory.getLogger(TableStatsHandlerTest.class);
+ Vertx vertx;
+ Server server;
+
+ @BeforeEach
+ void before() throws InterruptedException
+ {
+ Module testOverride = Modules.override(new TestModule())
+ .with(new TableStatsTestModule());
+ Injector injector = Guice.createInjector(Modules.override(new
MainModule())
+ .with(testOverride));
+ server = injector.getInstance(Server.class);
+ vertx = injector.getInstance(Vertx.class);
+ VertxTestContext context = new VertxTestContext();
+ server.start()
+ .onSuccess(s -> context.completeNow())
+ .onFailure(context::failNow);
+ context.awaitCompletion(5, TimeUnit.SECONDS);
+ }
+
+ @AfterEach
+ void after() throws InterruptedException
+ {
+ CountDownLatch closeLatch = new CountDownLatch(1);
+ server.close().onSuccess(res -> closeLatch.countDown());
Review Comment:
Can you use `TestResourceReaper` to tear down resources?
The class will be available once you rebase.
```suggestion
TestResourceReaper.create()
.with(server)
.with(vertx)
.close()
.onSuccess(res -> closeLatch.countDown());
```
--
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
