This is an automated email from the ASF dual-hosted git repository.
frankgh pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push:
new 56e2bb0f CASSSIDECAR-349: Thread race issue when adding excluded
metric in FilteringMetricRegistry (#263)
56e2bb0f is described below
commit 56e2bb0fd63785768039b28c35eed3c0fb6c1b4a
Author: Francisco Guerrero <[email protected]>
AuthorDate: Mon Sep 29 14:03:23 2025 -0700
CASSSIDECAR-349: Thread race issue when adding excluded metric in
FilteringMetricRegistry (#263)
Patch by Francisco Guerrero; reviewed by Yifan Cai for CASSSIDECAR-349
---
CHANGES.txt | 1 +
.../sidecar/metrics/FilteringMetricRegistry.java | 14 +++++++++++---
.../metrics/FilteringMetricRegistryTest.java | 22 ++++++++++++++++++++--
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 35e85a5d..07c4e007 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,6 @@
0.3.0
-----
+ * Thread race issue when adding excluded metric in FilteringMetricRegistry
(CASSSIDECAR-349)
* Add support for stateless JWT authentication using public keys
(CASSSIDECAR-334)
* Improve FilteringMetricRegistry implementation (CASSSIDECAR-347)
* Add lifecycle APIs for starting and stopping Cassandra (CASSSIDECAR-266)
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistry.java
b/server/src/main/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistry.java
index ddebb085..835363a8 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistry.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistry.java
@@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -283,15 +284,22 @@ public class FilteringMetricRegistry extends
MetricRegistry
*/
private Metric addExcludedMetricIfNotExists(String name, Function<String,
? extends Metric> mappingFunction)
{
- return excludedMetrics.computeIfAbsent(name, k -> {
+ AtomicBoolean registeredExcludedMetric = new AtomicBoolean(false);
+ Metric excludedMetric = excludedMetrics.computeIfAbsent(name, k -> {
+ registeredExcludedMetric.set(true);
+ return mappingFunction.apply(k);
+ });
+
+ if (registeredExcludedMetric.get())
+ {
// allMetrics needs to be recomputed when an excluded metric is
registered
synchronized (this)
{
allMetrics = null;
}
+ }
- return mappingFunction.apply(k);
- });
+ return excludedMetric;
}
/**
diff --git
a/server/src/test/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistryTest.java
b/server/src/test/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistryTest.java
index d8585374..9b8aa0c0 100644
---
a/server/src/test/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistryTest.java
+++
b/server/src/test/java/org/apache/cassandra/sidecar/metrics/FilteringMetricRegistryTest.java
@@ -21,13 +21,16 @@ package org.apache.cassandra.sidecar.metrics;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
import java.util.regex.Pattern;
import org.junit.jupiter.api.AfterEach;
@@ -62,6 +65,14 @@ import static
org.assertj.core.api.Assertions.assertThatThrownBy;
class FilteringMetricRegistryTest
{
private static final MetricRegistry NO_OP_METRIC_REGISTRY = new
NoopMetricRegistry();
+ private static final List<BiConsumer<FilteringMetricRegistry, String>>
METRIC_REGISTRY_STRING_BI_CONSUMER = List.of(
+ (r, metricName) -> r.register(metricName, new ThroughputMeter()),
+ FilteringMetricRegistry::meter,
+ FilteringMetricRegistry::gauge,
+ FilteringMetricRegistry::counter,
+ FilteringMetricRegistry::histogram,
+ FilteringMetricRegistry::timer
+ );
@TempDir
private Path confPath;
@@ -338,11 +349,13 @@ class FilteringMetricRegistryTest
pool.submit(() -> {
try
{
+ String metricName = "metric_" + finalI + "_" + ((finalI %
2 == 0) ? "even" : "odd");
+ BiConsumer<FilteringMetricRegistry, String> biConsumer =
registerRandomMetric();
// Invoke register roughly at the same time
latch.countDown();
latch.await();
- registry.register("testMetricThroughputMeter_" + finalI +
"_" + ((finalI % 2 == 0) ? "even" : "odd"), new ThroughputMeter());
+ biConsumer.accept(registry, metricName);
assertThat(registry.getMetrics()).isNotEmpty();
}
catch (InterruptedException e)
@@ -365,7 +378,7 @@ class FilteringMetricRegistryTest
Set<String> allMetricNames = allMetrics.keySet();
for (int i = 0; i < nThreads; i++)
{
- String expectedMetricName = "testMetricThroughputMeter_" + i + "_"
+ ((i % 2 == 0) ? "even" : "odd");
+ String expectedMetricName = "metric_" + i + "_" + ((i % 2 == 0) ?
"even" : "odd");
assertThat(allMetricNames).as("Expected metric %s",
expectedMetricName).contains(expectedMetricName);
}
}
@@ -424,4 +437,9 @@ class FilteringMetricRegistryTest
assertThat(allMetrics).as("About half the metrics are
removed").hasSize(nThreads / 2);
assertThat(registry.getIncludedMetrics()).hasSize(nThreads / 2);
}
+
+ BiConsumer<FilteringMetricRegistry, String> registerRandomMetric()
+ {
+ return
METRIC_REGISTRY_STRING_BI_CONSUMER.get(ThreadLocalRandom.current().nextInt(METRIC_REGISTRY_STRING_BI_CONSUMER.size()));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
