diff --git a/spectator-api/src/main/java/com/netflix/spectator/api/patterns/CardinalityLimiters.java b/spectator-api/src/main/java/com/netflix/spectator/api/patterns/CardinalityLimiters.java index 14e3f185f..fdfca98bb 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/api/patterns/CardinalityLimiters.java +++ b/spectator-api/src/main/java/com/netflix/spectator/api/patterns/CardinalityLimiters.java @@ -193,8 +193,7 @@ private void add(String s) { // Lock to keep hashmap consistent with counter for remaining lock.lock(); try { - if (remaining.get() > 0) { - values.put(s, s); + if (remaining.get() > 0 && values.put(s, s) == null) { remaining.decrementAndGet(); } } finally { diff --git a/spectator-api/src/test/java/com/netflix/spectator/api/patterns/CardinalityLimitersConcurrencyTest.java b/spectator-api/src/test/java/com/netflix/spectator/api/patterns/CardinalityLimitersConcurrencyTest.java new file mode 100644 index 000000000..7807c5b99 --- /dev/null +++ b/spectator-api/src/test/java/com/netflix/spectator/api/patterns/CardinalityLimitersConcurrencyTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2014-2024 Netflix, Inc. + *
+ * Licensed 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 com.netflix.spectator.api.patterns;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import static com.netflix.spectator.api.patterns.CardinalityLimiters.OTHERS;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class CardinalityLimitersConcurrencyTest {
+
+ private final ExecutorService pool = Executors.newFixedThreadPool(100);
+
+ @Test
+ void firstLimiterWhenContentionSizeIsRespected() throws InterruptedException, ExecutionException {
+ Function> task = () -> IntStream
+ .rangeClosed(1, 100)
+ .mapToObj(j -> limiter.apply(String.valueOf(j))).collect(Collectors.toList());
+ List
> handle : handles) {
+ handle.get();
+ }
+
+ // then: expect each number to have been registered despite the contention
+ // since the bound has not been reached. (i.e OTHERS must not be present)
+ List