From 5210c3e12f406b1631c17240f318f2f240bdb602 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Mon, 29 Sep 2025 19:50:16 -0400 Subject: [PATCH 1/6] CNDB-7197: Added SAI_INDEX_METRICS_ENABLED to be able to enable/disable IndexMetrics --- .../config/CassandraRelevantProperties.java | 6 ++ .../cassandra/index/sai/IndexContext.java | 14 +-- .../sai/disk/v1/MemtableIndexWriter.java | 9 +- .../index/sai/disk/v1/SSTableIndexWriter.java | 3 +- .../index/sai/plan/QueryController.java | 6 +- .../index/sai/metrics/IndexMetricsTest.java | 95 ++++++++++++++++++- 6 files changed, 119 insertions(+), 14 deletions(-) diff --git a/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java b/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java index 464ffe07d767..903373796547 100644 --- a/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java +++ b/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java @@ -586,6 +586,12 @@ public enum CassandraRelevantProperties */ SAI_QUERY_KIND_PER_QUERY_METRICS_ENABLED("cassandra.sai.metrics.query_kind.per_query.enabled", "false"), + /** + * Whether to enable SAI index metrics such as memtable flush metrics, compaction metrics, and disk usage metrics. + * These metrics include timers, histograms, counters, and gauges for index operations. + */ + SAI_INDEX_METRICS_ENABLED("cassandra.sai.metrics.index.enabled", "true"), + /** * If true, while creating or altering schema, NetworkTopologyStrategy won't check if the DC exists. * This is to remain compatible with older workflows that first change the replication before adding the nodes. diff --git a/src/java/org/apache/cassandra/index/sai/IndexContext.java b/src/java/org/apache/cassandra/index/sai/IndexContext.java index f6eb4a974f94..9bec4e131cb1 100644 --- a/src/java/org/apache/cassandra/index/sai/IndexContext.java +++ b/src/java/org/apache/cassandra/index/sai/IndexContext.java @@ -38,7 +38,6 @@ import org.slf4j.LoggerFactory; import io.github.jbellis.jvector.vector.VectorSimilarityFunction; -import org.apache.cassandra.config.CassandraRelevantProperties; import org.apache.cassandra.cql3.Operator; import org.apache.cassandra.cql3.statements.schema.IndexTarget; import org.apache.cassandra.db.ClusteringComparator; @@ -73,7 +72,6 @@ import org.apache.cassandra.index.sai.iterators.KeyRangeUnionIterator; import org.apache.cassandra.index.sai.memory.MemtableIndex; import org.apache.cassandra.index.sai.memory.MemtableKeyRangeIterator; -import org.apache.cassandra.index.sai.memory.TrieMemtableIndex; import org.apache.cassandra.index.sai.metrics.ColumnQueryMetrics; import org.apache.cassandra.index.sai.metrics.IndexMetrics; import org.apache.cassandra.index.sai.plan.Expression; @@ -93,6 +91,7 @@ import org.apache.cassandra.utils.concurrent.OpOrder; import static org.apache.cassandra.config.CassandraRelevantProperties.SAI_INDEX_READS_DISABLED; +import static org.apache.cassandra.config.CassandraRelevantProperties.SAI_INDEX_METRICS_ENABLED; import static org.apache.cassandra.config.CassandraRelevantProperties.VALIDATE_MAX_TERM_SIZE_AT_COORDINATOR; /** @@ -190,7 +189,7 @@ public IndexContext(@Nonnull String keyspace, this.vectorSimilarityFunction = indexWriterConfig.getSimilarityFunction(); this.hasEuclideanSimilarityFunc = vectorSimilarityFunction == VectorSimilarityFunction.EUCLIDEAN; - this.indexMetrics = new IndexMetrics(this); + this.indexMetrics = SAI_INDEX_METRICS_ENABLED.getBoolean() ? new IndexMetrics(this) : null; this.columnQueryMetrics = isVector() ? new ColumnQueryMetrics.VectorIndexMetrics(keyspace, table, getIndexName()) : isLiteral() ? new ColumnQueryMetrics.TrieIndexMetrics(keyspace, table, getIndexName()) : new ColumnQueryMetrics.BKDIndexMetrics(keyspace, table, getIndexName()); @@ -298,7 +297,8 @@ public void index(DecoratedKey key, Row row, Memtable memtable, OpOrder.Group op ByteBuffer value = getValueOf(key, row, FBUtilities.nowInSeconds()); target.index(key, row.clustering(), value, memtable, opGroup); } - indexMetrics.memtableIndexWriteLatency.update(System.nanoTime() - start, TimeUnit.NANOSECONDS); + if (indexMetrics != null) + indexMetrics.memtableIndexWriteLatency.update(System.nanoTime() - start, TimeUnit.NANOSECONDS); } /** @@ -690,8 +690,10 @@ public void invalidate(boolean obsolete) dropped = true; liveMemtables.clear(); viewManager.invalidate(obsolete); - indexMetrics.release(); - columnQueryMetrics.release(); + if (indexMetrics != null) + indexMetrics.release(); + if (columnQueryMetrics != null) + columnQueryMetrics.release(); analyzerFactory.close(); if (queryAnalyzerFactory != analyzerFactory) diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java index a2cd5f23edd1..a074350836df 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java @@ -138,7 +138,8 @@ public void complete(Stopwatch stopwatch) throws IOException catch (Throwable t) { logger.error(perIndexComponents.logMessage("Error while flushing index {}"), t.getMessage(), t); - indexContext().getIndexMetrics().memtableIndexFlushErrors.inc(); + if (indexContext().getIndexMetrics() != null) + indexContext().getIndexMetrics().memtableIndexFlushErrors.inc(); throw t; } @@ -263,7 +264,8 @@ private void completeIndexFlush(long cellCount, long startTime, Stopwatch stopwa { perIndexComponents.markComplete(); - indexContext().getIndexMetrics().memtableIndexFlushCount.inc(); + if (indexContext().getIndexMetrics() != null) + indexContext().getIndexMetrics().memtableIndexFlushCount.inc(); long elapsedTime = stopwatch.elapsed(TimeUnit.MILLISECONDS); @@ -273,6 +275,7 @@ private void completeIndexFlush(long cellCount, long startTime, Stopwatch stopwa elapsedTime - startTime, elapsedTime); - indexContext().getIndexMetrics().memtableFlushCellsPerSecond.update((long) (cellCount * 1000.0 / Math.max(1, elapsedTime - startTime))); + if (indexContext().getIndexMetrics() != null) + indexContext().getIndexMetrics().memtableFlushCellsPerSecond.update((long) (cellCount * 1000.0 / Math.max(1, elapsedTime - startTime))); } } diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java index 7989ecb9086c..261ca878ea36 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java @@ -352,7 +352,8 @@ private void flushSegment() throws IOException logger.error("Failed to build index for SSTable {}", perIndexComponents.descriptor(), t); perIndexComponents.forceDeleteAllComponents(); - indexContext.getIndexMetrics().segmentFlushErrors.inc(); + if (indexContext.getIndexMetrics() != null) + indexContext.getIndexMetrics().segmentFlushErrors.inc(); throw t; } diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java index 6b10fb2f01e3..9cb65358c8c5 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java +++ b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java @@ -378,8 +378,10 @@ private void updateIndexMetricsQueriesCount(Plan plan) queriedIndexesContexts.add(indexContext); return Plan.ControlFlow.Continue; }); - queriedIndexesContexts.forEach(indexContext -> - indexContext.getIndexMetrics().queriesCount.inc()); + queriedIndexesContexts.forEach(indexContext -> { + if (indexContext.getIndexMetrics() != null) + indexContext.getIndexMetrics().queriesCount.inc(); + }); } Plan buildPlan() diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java index 057d71b76fa2..27181dc30fd3 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java @@ -19,14 +19,17 @@ import java.util.concurrent.TimeUnit; +import org.apache.cassandra.config.CassandraRelevantProperties; import org.junit.Test; import com.datastax.driver.core.ResultSet; import org.apache.cassandra.utils.Throwables; +import javax.management.InstanceNotFoundException; +import javax.management.ObjectName; + import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; public class IndexMetricsTest extends AbstractMetricsTest { @@ -144,6 +147,94 @@ public void testMetricsThroughWriteLifecycle() waitForHistogramMeanBetween(objectName("CompactionSegmentCellsPerSecond", KEYSPACE, table, index, "IndexMetrics"), 1.0, 1000000.0); } + @Test + public void testIndexMetricsEnabledAndDisabled() + { + testIndexMetrics(true); + testIndexMetrics(false); + } + + private void testIndexMetrics(boolean metricsEnabled) + { + // Set the property before creating any indexes + CassandraRelevantProperties.SAI_INDEX_METRICS_ENABLED.setBoolean(metricsEnabled); + + try + { + String table = createTable("CREATE TABLE %s (ID1 TEXT PRIMARY KEY, v1 INT, v2 TEXT) WITH compaction = " + + "{'class' : 'SizeTieredCompactionStrategy', 'enabled' : false }"); + String index = createIndex("CREATE CUSTOM INDEX IF NOT EXISTS ON %s (v1) USING 'StorageAttachedIndex'"); + + // Test all Gauge metrics + assertMetricExistsIfEnabled(metricsEnabled, "SSTableCellCount", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "LiveMemtableIndexWriteCount", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "DiskUsedBytes", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "MemtableOnHeapIndexBytes", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "MemtableOffHeapIndexBytes", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "IndexFileCacheBytes", table, index); + + // Test all Counter metrics + assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushCount", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "CompactionCount", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushErrors", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentFlushErrors", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "QueriesCount", table, index); + + // Test all Histogram metrics + assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushCellsPerSecond", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "SegmentsPerCompaction", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentCellsPerSecond", table, index); + assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentBytesPerSecond", table, index); + + // Test Timer metrics + assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexWriteLatency", table, index); + } + finally + { + // Reset property to default + CassandraRelevantProperties.SAI_INDEX_METRICS_ENABLED.setBoolean(true); + } + } + + private void assertMetricExistsIfEnabled(boolean shouldExist, String metricName, String table, String index) + { + ObjectName name = objectName(metricName, KEYSPACE, table, index, "IndexMetrics"); + + if (shouldExist) + assertMetricExists(name, metricName); + else + assertMetricDoesNotExist(name, metricName); + } + + private void assertMetricExists(ObjectName name, String metricName) + { + try + { + getMetricValue(name); + // If we get here without exception, the metric exists - that's what we want + } + catch (Exception e) + { + fail("Expected metric " + metricName + " to be registered when metrics are enabled, but got: " + e.getMessage()); + } + } + + private void assertMetricDoesNotExist(ObjectName name, String metricName) + { + try + { + getMetricValue(name); + fail("Expected metric " + metricName + " to not be registered when metrics are disabled"); + } + catch (Exception e) + { + // Expected - metrics should not be accessible when disabled + assertTrue("Expected InstanceNotFoundException for " + metricName + " but got: " + e.getClass().getSimpleName(), + e.getCause() instanceof InstanceNotFoundException || + e instanceof InstanceNotFoundException); + } + } + private void assertIndexQueryCount(String index, long expectedCount) { assertEquals(expectedCount, From 0e76b138f65d915ceba99a84e26921ad27f6c150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20de=20la=20Pe=C3=B1a?= Date: Tue, 30 Sep 2025 16:01:17 +0100 Subject: [PATCH 2/6] CNDB-7197: Review feedback addressed - make getIndexMetrics return Optional --- .../cassandra/index/sai/IndexContext.java | 11 ++--- .../sai/disk/v1/MemtableIndexWriter.java | 11 ++--- .../index/sai/disk/v1/SSTableIndexWriter.java | 22 ++++----- .../index/sai/plan/QueryController.java | 3 +- .../apache/cassandra/index/sai/SAITester.java | 5 ++ .../index/sai/metrics/IndexMetricsTest.java | 46 ++++--------------- 6 files changed, 34 insertions(+), 64 deletions(-) diff --git a/src/java/org/apache/cassandra/index/sai/IndexContext.java b/src/java/org/apache/cassandra/index/sai/IndexContext.java index 9bec4e131cb1..eab6c2e7e59e 100644 --- a/src/java/org/apache/cassandra/index/sai/IndexContext.java +++ b/src/java/org/apache/cassandra/index/sai/IndexContext.java @@ -19,11 +19,7 @@ package org.apache.cassandra.index.sai; import java.nio.ByteBuffer; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.Objects; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; @@ -139,6 +135,7 @@ public class IndexContext private final ConcurrentMap liveMemtables = new ConcurrentHashMap<>(); private final IndexViewManager viewManager; + @Nullable private final IndexMetrics indexMetrics; private final ColumnQueryMetrics columnQueryMetrics; private final IndexWriterConfig indexWriterConfig; @@ -234,9 +231,9 @@ public ClusteringComparator comparator() return clusteringComparator; } - public IndexMetrics getIndexMetrics() + public Optional getIndexMetrics() { - return indexMetrics; + return Optional.ofNullable(indexMetrics); } public ColumnQueryMetrics getColumnQueryMetrics() diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java index a074350836df..fe8d85c43dd4 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java @@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Stopwatch; +import org.apache.cassandra.index.sai.metrics.IndexMetrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -138,8 +139,7 @@ public void complete(Stopwatch stopwatch) throws IOException catch (Throwable t) { logger.error(perIndexComponents.logMessage("Error while flushing index {}"), t.getMessage(), t); - if (indexContext().getIndexMetrics() != null) - indexContext().getIndexMetrics().memtableIndexFlushErrors.inc(); + indexContext().getIndexMetrics().ifPresent(m -> m.memtableIndexFlushErrors.inc()); throw t; } @@ -264,8 +264,7 @@ private void completeIndexFlush(long cellCount, long startTime, Stopwatch stopwa { perIndexComponents.markComplete(); - if (indexContext().getIndexMetrics() != null) - indexContext().getIndexMetrics().memtableIndexFlushCount.inc(); + indexContext().getIndexMetrics().ifPresent(m -> m.memtableIndexFlushCount.inc()); long elapsedTime = stopwatch.elapsed(TimeUnit.MILLISECONDS); @@ -275,7 +274,7 @@ private void completeIndexFlush(long cellCount, long startTime, Stopwatch stopwa elapsedTime - startTime, elapsedTime); - if (indexContext().getIndexMetrics() != null) - indexContext().getIndexMetrics().memtableFlushCellsPerSecond.update((long) (cellCount * 1000.0 / Math.max(1, elapsedTime - startTime))); + indexContext().getIndexMetrics() + .ifPresent(m -> m.memtableFlushCellsPerSecond.update((long) (cellCount * 1000.0 / Math.max(1, elapsedTime - startTime)))); } } diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java index 261ca878ea36..4cb7e8fd6cf4 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java @@ -191,12 +191,11 @@ public void complete(Stopwatch stopwatch) throws IOException } finally { - if (indexContext.getIndexMetrics() != null) - { - indexContext.getIndexMetrics().segmentsPerCompaction.update(segments.size()); + indexContext.getIndexMetrics().ifPresent(m -> { + m.segmentsPerCompaction.update(segments.size()); segments.clear(); - indexContext.getIndexMetrics().compactionCount.inc(); - } + m.compactionCount.inc(); + }); } } @@ -325,12 +324,12 @@ private void flushSegment() throws IOException segments.add(segmentMetadata); double rowCount = segmentMetadata.numRows; - if (indexContext.getIndexMetrics() != null) - indexContext.getIndexMetrics().compactionSegmentCellsPerSecond.update((long)(rowCount / flushMillis * 1000.0)); - double segmentBytes = segmentMetadata.componentMetadatas.indexSize(); - if (indexContext.getIndexMetrics() != null) - indexContext.getIndexMetrics().compactionSegmentBytesPerSecond.update((long)(segmentBytes / flushMillis * 1000.0)); + + indexContext.getIndexMetrics().ifPresent(m -> { + m.compactionSegmentCellsPerSecond.update((long)(rowCount / flushMillis * 1000.0)); + m.compactionSegmentBytesPerSecond.update((long)(segmentBytes / flushMillis * 1000.0)); + }); logger.debug("Flushed segment with {} cells for a total of {} in {} ms for index {} with starting row id {} for sstable {}", (long) rowCount, FBUtilities.prettyPrintMemory((long) segmentBytes), flushMillis, indexContext.getIndexName(), @@ -352,8 +351,7 @@ private void flushSegment() throws IOException logger.error("Failed to build index for SSTable {}", perIndexComponents.descriptor(), t); perIndexComponents.forceDeleteAllComponents(); - if (indexContext.getIndexMetrics() != null) - indexContext.getIndexMetrics().segmentFlushErrors.inc(); + indexContext.getIndexMetrics().ifPresent(m -> m.segmentFlushErrors.inc()); throw t; } diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java index 9cb65358c8c5..b7d506b11da2 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java +++ b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java @@ -379,8 +379,7 @@ private void updateIndexMetricsQueriesCount(Plan plan) return Plan.ControlFlow.Continue; }); queriedIndexesContexts.forEach(indexContext -> { - if (indexContext.getIndexMetrics() != null) - indexContext.getIndexMetrics().queriesCount.inc(); + indexContext.getIndexMetrics().ifPresent(m -> m.queriesCount.inc()); }); } diff --git a/test/unit/org/apache/cassandra/index/sai/SAITester.java b/test/unit/org/apache/cassandra/index/sai/SAITester.java index cd7bd191eb06..6a1c8ebb728e 100644 --- a/test/unit/org/apache/cassandra/index/sai/SAITester.java +++ b/test/unit/org/apache/cassandra/index/sai/SAITester.java @@ -466,6 +466,11 @@ protected Object getMetricValue(ObjectName metricObjectName) return metricValue; } + protected void assertMetricExists(ObjectName name) + { + Assertions.assertThatNoException().isThrownBy(() -> getMetricValue(name)); + } + protected void assertMetricDoesNotExist(ObjectName name) { Assertions.assertThatThrownBy(() -> getMetricValue(name)).hasRootCauseInstanceOf(InstanceNotFoundException.class); diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java index 27181dc30fd3..88484537358d 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java @@ -20,6 +20,7 @@ import java.util.concurrent.TimeUnit; import org.apache.cassandra.config.CassandraRelevantProperties; +import org.assertj.core.api.Assertions; import org.junit.Test; import com.datastax.driver.core.ResultSet; @@ -158,13 +159,13 @@ private void testIndexMetrics(boolean metricsEnabled) { // Set the property before creating any indexes CassandraRelevantProperties.SAI_INDEX_METRICS_ENABLED.setBoolean(metricsEnabled); - + try { String table = createTable("CREATE TABLE %s (ID1 TEXT PRIMARY KEY, v1 INT, v2 TEXT) WITH compaction = " + "{'class' : 'SizeTieredCompactionStrategy', 'enabled' : false }"); String index = createIndex("CREATE CUSTOM INDEX IF NOT EXISTS ON %s (v1) USING 'StorageAttachedIndex'"); - + // Test all Gauge metrics assertMetricExistsIfEnabled(metricsEnabled, "SSTableCellCount", table, index); assertMetricExistsIfEnabled(metricsEnabled, "LiveMemtableIndexWriteCount", table, index); @@ -172,20 +173,20 @@ private void testIndexMetrics(boolean metricsEnabled) assertMetricExistsIfEnabled(metricsEnabled, "MemtableOnHeapIndexBytes", table, index); assertMetricExistsIfEnabled(metricsEnabled, "MemtableOffHeapIndexBytes", table, index); assertMetricExistsIfEnabled(metricsEnabled, "IndexFileCacheBytes", table, index); - + // Test all Counter metrics assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushCount", table, index); assertMetricExistsIfEnabled(metricsEnabled, "CompactionCount", table, index); assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushErrors", table, index); assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentFlushErrors", table, index); assertMetricExistsIfEnabled(metricsEnabled, "QueriesCount", table, index); - + // Test all Histogram metrics assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexFlushCellsPerSecond", table, index); assertMetricExistsIfEnabled(metricsEnabled, "SegmentsPerCompaction", table, index); assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentCellsPerSecond", table, index); assertMetricExistsIfEnabled(metricsEnabled, "CompactionSegmentBytesPerSecond", table, index); - + // Test Timer metrics assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexWriteLatency", table, index); } @@ -199,40 +200,11 @@ private void testIndexMetrics(boolean metricsEnabled) private void assertMetricExistsIfEnabled(boolean shouldExist, String metricName, String table, String index) { ObjectName name = objectName(metricName, KEYSPACE, table, index, "IndexMetrics"); - + if (shouldExist) - assertMetricExists(name, metricName); + assertMetricExists(name); else - assertMetricDoesNotExist(name, metricName); - } - - private void assertMetricExists(ObjectName name, String metricName) - { - try - { - getMetricValue(name); - // If we get here without exception, the metric exists - that's what we want - } - catch (Exception e) - { - fail("Expected metric " + metricName + " to be registered when metrics are enabled, but got: " + e.getMessage()); - } - } - - private void assertMetricDoesNotExist(ObjectName name, String metricName) - { - try - { - getMetricValue(name); - fail("Expected metric " + metricName + " to not be registered when metrics are disabled"); - } - catch (Exception e) - { - // Expected - metrics should not be accessible when disabled - assertTrue("Expected InstanceNotFoundException for " + metricName + " but got: " + e.getClass().getSimpleName(), - e.getCause() instanceof InstanceNotFoundException || - e instanceof InstanceNotFoundException); - } + assertMetricDoesNotExist(name); } private void assertIndexQueryCount(String index, long expectedCount) From 30a4409ba269d41a2166c885e2482d0baddb288b Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Tue, 30 Sep 2025 13:18:32 -0400 Subject: [PATCH 3/6] CNDB-7197: Remove unused imports --- .../apache/cassandra/index/sai/metrics/IndexMetricsTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java index 88484537358d..d45c03d756f8 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java @@ -20,13 +20,11 @@ import java.util.concurrent.TimeUnit; import org.apache.cassandra.config.CassandraRelevantProperties; -import org.assertj.core.api.Assertions; import org.junit.Test; import com.datastax.driver.core.ResultSet; import org.apache.cassandra.utils.Throwables; -import javax.management.InstanceNotFoundException; import javax.management.ObjectName; import static org.assertj.core.api.Assertions.assertThatThrownBy; From b4d87c725d91d218425602a8e447c858a3009f13 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Tue, 30 Sep 2025 16:13:11 -0400 Subject: [PATCH 4/6] CNDB-7197: Add test for MemtableIndexFlushErrors --- .../sai/disk/v1/MemtableIndexWriter.java | 1 - .../index/sai/metrics/IndexMetricsTest.java | 38 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java index fe8d85c43dd4..f06bc9fe7431 100644 --- a/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java +++ b/src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java @@ -24,7 +24,6 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Stopwatch; -import org.apache.cassandra.index.sai.metrics.IndexMetrics; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java index d45c03d756f8..cb201d2485bf 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java @@ -30,6 +30,10 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.*; +import org.apache.cassandra.inject.Injection; +import org.apache.cassandra.inject.Injections; +import org.apache.cassandra.index.sai.disk.v1.MemtableIndexWriter; + public class IndexMetricsTest extends AbstractMetricsTest { @@ -251,4 +255,38 @@ public void testQueriesCount() assertIndexQueryCount(indexV2, 2L); assertIndexQueryCount(indexV3, 1L); } + + @Test + public void testMemtableIndexFlushErrorIncrementsMetric() throws Throwable + { + String table = createTable("CREATE TABLE %s (ID1 TEXT PRIMARY KEY, v1 INT, v2 TEXT) WITH compaction = " + + "{'class' : 'SizeTieredCompactionStrategy', 'enabled' : false }"); + String index = createIndex("CREATE CUSTOM INDEX IF NOT EXISTS ON %s (v1) USING 'StorageAttachedIndex'"); + + // Write some data to ensure there is something to flush + execute("INSERT INTO %s (id1, v1, v2) VALUES ('0', 0, '0')"); + + assertEquals(0L, getMetricValue(objectName("MemtableIndexFlushErrors", KEYSPACE, table, index, "IndexMetrics"))); + + // Inject a failure at the entry of MemtableIndexWriter#flush(...) to force a flush error + Injection failure = newFailureOnEntry("sai_memtable_flush_error", MemtableIndexWriter.class, "flush", RuntimeException.class); + Injections.inject(failure); + + try + { + // Trigger a flush, which should hit the injected failure + flush(KEYSPACE, table); + } + catch (Throwable ignored) + { + // Expected due to injected failure + } + finally + { + failure.disable(); + } + + // Verify the memtable index flush error metric is incremented + assertEquals(1L, getMetricValue(objectName("MemtableIndexFlushErrors", KEYSPACE, table, index, "IndexMetrics"))); + } } From b12aed85923eb339f2ffa0df59c0b7781ebed443 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Tue, 30 Sep 2025 16:16:32 -0400 Subject: [PATCH 5/6] CNDB-7197: Remove useless curly braces around statement as per Sonar recommendation --- .../org/apache/cassandra/index/sai/plan/QueryController.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java index b7d506b11da2..bf3bf49ec649 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java +++ b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java @@ -378,9 +378,8 @@ private void updateIndexMetricsQueriesCount(Plan plan) queriedIndexesContexts.add(indexContext); return Plan.ControlFlow.Continue; }); - queriedIndexesContexts.forEach(indexContext -> { - indexContext.getIndexMetrics().ifPresent(m -> m.queriesCount.inc()); - }); + queriedIndexesContexts.forEach(indexContext -> indexContext.getIndexMetrics() + .ifPresent(m -> m.queriesCount.inc())); } Plan buildPlan() From 3b90f88324c2bc79c218a7c4021e8cc12134f930 Mon Sep 17 00:00:00 2001 From: Ekaterina Dimitrova Date: Wed, 1 Oct 2025 15:23:30 -0400 Subject: [PATCH 6/6] CNDB-7197: Improve test coverage --- .../cassandra/index/sai/metrics/IndexMetricsTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java index cb201d2485bf..342d20f1b4e3 100644 --- a/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java +++ b/test/unit/org/apache/cassandra/index/sai/metrics/IndexMetricsTest.java @@ -191,6 +191,14 @@ private void testIndexMetrics(boolean metricsEnabled) // Test Timer metrics assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexWriteLatency", table, index); + + // Test indexing operations to ensure null indexMetrics is handled gracefully + execute("INSERT INTO %s (id1, v1, v2) VALUES ('0', 0, '0')"); + execute("INSERT INTO %s (id1, v1, v2) VALUES ('1', 1, '1')"); + execute("INSERT INTO %s (id1, v1, v2) VALUES ('2', 2, '2')"); + + // Verify MemtableIndexWriteLatency metric behavior after indexing operations + assertMetricExistsIfEnabled(metricsEnabled, "MemtableIndexWriteLatency", table, index); } finally {