From 762020619ba06e4761e84e2278765723e8075052 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 9 May 2024 14:25:50 +0300 Subject: [PATCH 1/2] [test] Fix flaky DefaultPooledConnectionProviderTest#testDisposeInactivePoolsInBackground There are random DefaultPooledConnectionProviderTest#testDisposeInactivePoolsInBackground failures DefaultPooledConnectionProviderTest > testDisposeInactivePoolsInBackground(boolean, boolean, boolean) > [5] enableEvictInBackground=true, isHttp2=false, isBuiltInMetrics=false FAILED org.opentest4j.AssertionFailedError: expected: true but was: false at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at reactor.netty.resources.DefaultPooledConnectionProviderTest.testDisposeInactivePoolsInBackground(DefaultPooledConnectionProviderTest.java:729) DefaultPooledConnectionProviderTest > testDisposeInactivePoolsInBackground(boolean, boolean, boolean) > [8] enableEvictInBackground=true, isHttp2=true, isBuiltInMetrics=true FAILED org.opentest4j.AssertionFailedError: expected: null but was: io.micrometer.core.instrument.internal.DefaultGauge@78e01239 at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at reactor.netty.resources.DefaultPooledConnectionProviderTest.testDisposeInactivePoolsInBackground(DefaultPooledConnectionProviderTest.java:733) --- .../DefaultPooledConnectionProviderTest.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java index 57930a259..4139e7beb 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java @@ -669,12 +669,20 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole } MeterRegistrarImpl meterRegistrar; - String metricsName = ""; + CountDownLatch meterRemoved = new CountDownLatch(1); + String metricsName = CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS; + String metricsTagName = ""; if (isBuiltInMetrics) { meterRegistrar = null; builder.metrics(true); - metricsName = isHttp2 ? "http2.testDisposeInactivePoolsInBackground" : "testDisposeInactivePoolsInBackground"; + metricsTagName = isHttp2 ? "http2.testDisposeInactivePoolsInBackground" : "testDisposeInactivePoolsInBackground"; + + registry.config().onMeterRemoved(meter -> { + if (metricsName.equals(meter.getId().getName())) { + meterRemoved.countDown(); + } + }); } else { meterRegistrar = new MeterRegistrarImpl(); @@ -711,10 +719,14 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole assertThat(meterRegistrar.registered.get()).isTrue(); } else { - assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNotNull(); + assertGauge(registry, metricsName, NAME, metricsTagName).isNotNull(); } if (enableEvictInBackground) { + if (meterRegistrar != null) { + provider.onDispose.subscribe(null, null, meterRemoved::countDown); + } + assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); } @@ -726,14 +738,18 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole assertThat(provider.isDisposed()).isEqualTo(enableEvictInBackground); if (meterRegistrar != null) { + if (enableEvictInBackground) { + assertThat(meterRemoved.await(30, TimeUnit.SECONDS)).isTrue(); + } assertThat(meterRegistrar.deRegistered.get()).isEqualTo(enableEvictInBackground); } else { if (enableEvictInBackground) { - assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNull(); + assertThat(meterRemoved.await(30, TimeUnit.SECONDS)).isTrue(); + assertGauge(registry, metricsName, NAME, metricsTagName).isNull(); } else { - assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNotNull(); + assertGauge(registry, metricsName, NAME, metricsTagName).isNotNull(); } } } From b93fe893b20a355e5598228f9509e327289d0e8f Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 9 May 2024 15:06:05 +0300 Subject: [PATCH 2/2] In addition to previous commit --- .../resources/DefaultPooledConnectionProviderTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java index 4139e7beb..cf6175a00 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java @@ -723,10 +723,6 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole } if (enableEvictInBackground) { - if (meterRegistrar != null) { - provider.onDispose.subscribe(null, null, meterRemoved::countDown); - } - assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); } @@ -739,7 +735,7 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole assertThat(provider.isDisposed()).isEqualTo(enableEvictInBackground); if (meterRegistrar != null) { if (enableEvictInBackground) { - assertThat(meterRemoved.await(30, TimeUnit.SECONDS)).isTrue(); + assertThat(meterRegistrar.latch.await(30, TimeUnit.SECONDS)).isTrue(); } assertThat(meterRegistrar.deRegistered.get()).isEqualTo(enableEvictInBackground); } @@ -885,6 +881,7 @@ void testHttp2PoolAndGoAway() { static final class MeterRegistrarImpl implements ConnectionProvider.MeterRegistrar { AtomicBoolean registered = new AtomicBoolean(); AtomicBoolean deRegistered = new AtomicBoolean(); + final CountDownLatch latch = new CountDownLatch(1); MeterRegistrarImpl() { } @@ -897,6 +894,7 @@ public void registerMetrics(String poolName, String id, SocketAddress remoteAddr @Override public void deRegisterMetrics(String poolName, String id, SocketAddress remoteAddress) { deRegistered.compareAndSet(false, true); + latch.countDown(); } } }