From 5c1085337dc13627f59022e2562b606751c43909 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Thu, 17 Oct 2019 22:29:27 +0900 Subject: [PATCH] Fix a race condition in MeterRegistry.getOrCreateMeter() (#1628) This commit resolves a race condition between meter addition and composite meter's children addition via meter-added event listener. Closes gh-1622 --- .../core/instrument/MeterRegistry.java | 2 +- .../composite/CompositeMeterRegistryTest.java | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java index 88c8bd5b99..f9e4c7cf57 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java @@ -573,7 +573,6 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config, } m = builder.apply(mappedId, config); - meterMap = meterMap.plus(mappedId, m); Id synAssoc = originalId.syntheticAssociation(); if (synAssoc != null) { @@ -584,6 +583,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config, for (Consumer onAdd : meterAddedListeners) { onAdd.accept(m); } + meterMap = meterMap.plus(mappedId, m); } } } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java index a9b06a1971..3183edb965 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java @@ -26,6 +26,7 @@ import io.micrometer.core.instrument.step.StepMeterRegistry; import io.micrometer.core.instrument.step.StepRegistryConfig; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import reactor.core.publisher.Flux; import reactor.core.scheduler.Schedulers; @@ -34,6 +35,8 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -445,4 +448,31 @@ void stopTrackingMetersThatAreRemoved() { counting.publish(); assertThat(counting.count(functionCounter)).isEqualTo(1); } + + @RepeatedTest(100) + void meterRegistrationShouldWorkConcurrently() throws InterruptedException { + this.composite.add(this.simple); + + String meterName = "test.counter"; + String tagName = "test.tag"; + + int count = 10000; + int tagCount = 100; + ExecutorService executor = Executors.newFixedThreadPool(10); + for (int i = 0; i < count; i++) { + int tagValue = i % tagCount; + executor.execute(() -> { + Counter counter = Counter.builder(meterName).tag(tagName, String.valueOf(tagValue)) + .register(this.composite); + counter.increment(); + }); + } + executor.shutdown(); + assertThat(executor.awaitTermination(1L, TimeUnit.SECONDS)).isTrue(); + for (int i = 0; i < tagCount; i++) { + assertThat(this.composite.find(meterName).tag(tagName, String.valueOf(i)).counter().count()) + .isEqualTo(count / tagCount); + } + } + }