From 27ecd6ccc8b12b1cc86e3687998a6eabb4bd09b5 Mon Sep 17 00:00:00 2001 From: ggivo Date: Wed, 15 Jan 2025 12:44:30 +0200 Subject: [PATCH] Fix sporadic test failing with OOM (#4053) * Attempt to fix sporadic test failing with OOM Troubleshooting flaky test failing with OOM * Authx.close() in try/finally * Configure retention for uploaded heap dumps to 5days remove PrintFlagsFinal to avoid warning in buld logs * format TokenBasedAuthenticationUnitTests --------- Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> --- .github/workflows/test-on-docker.yml | 13 ++++ pom.xml | 1 + .../TokenBasedAuthenticationUnitTests.java | 74 +++++++++++-------- 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/.github/workflows/test-on-docker.yml b/.github/workflows/test-on-docker.yml index 985b9cc45b..e7f0912811 100644 --- a/.github/workflows/test-on-docker.yml +++ b/.github/workflows/test-on-docker.yml @@ -83,7 +83,20 @@ jobs: mvn -Dtest=$TESTS clean compile test fi env: + JVM_OPTS: "-XX:+HeapDumpOnOutOfMemoryError -XX:+ExitOnOutOfMemoryError -XX:HeapDumpPath=${{ runner.temp }}/heapdump-${{ matrix.redis_version }}.hprof" TESTS: ${{ github.event.inputs.specific_test || '' }} + - name: Upload Heap Dumps + if: failure() + uses: actions/upload-artifact@v4 + with: + name: heap-dumps-${{ matrix.redis_version }} + path: ${{ runner.temp }}/heapdump-${{ matrix.redis_version }}.hprof + retention-days: 5 + - name: Upload Surefire Dump File + uses: actions/upload-artifact@v3 + with: + name: surefire-dumpstream + path: target/surefire-reports/*.dumpstream - name: Publish Test Results uses: EnricoMi/publish-unit-test-result-action@v2 if: always() diff --git a/pom.xml b/pom.xml index add6b378ea..ca5f6aace4 100644 --- a/pom.xml +++ b/pom.xml @@ -234,6 +234,7 @@ maven-surefire-plugin ${maven.surefire.version} + ${JVM_OPTS} ${redis-hosts} diff --git a/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java b/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java index d3f02fb2c8..df223a76ee 100644 --- a/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java +++ b/src/test/java/redis/clients/jedis/authentication/TokenBasedAuthenticationUnitTests.java @@ -72,17 +72,18 @@ public void withExpirationRefreshRatio_testJedisAuthXManagerTriggersEvict() thro AuthXManager jedisAuthXManager = new AuthXManager(tokenManager); AtomicInteger numberOfEvictions = new AtomicInteger(0); - ConnectionPool pool = new ConnectionPool(hnp, + + try (ConnectionPool pool = new ConnectionPool(hnp, endpoint.getClientConfigBuilder().authXManager(jedisAuthXManager).build()) { @Override public void evict() throws Exception { numberOfEvictions.incrementAndGet(); super.evict(); } - }; - - await().pollInterval(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) - .until(numberOfEvictions::get, Matchers.greaterThanOrEqualTo(1)); + }) { + await().pollInterval(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) + .until(numberOfEvictions::get, Matchers.greaterThanOrEqualTo(1)); + } } public void withLowerRefreshBounds_testJedisAuthXManagerTriggersEvict() throws Exception { @@ -97,17 +98,18 @@ public void withLowerRefreshBounds_testJedisAuthXManagerTriggersEvict() throws E AuthXManager jedisAuthXManager = new AuthXManager(tokenManager); AtomicInteger numberOfEvictions = new AtomicInteger(0); - ConnectionPool pool = new ConnectionPool(endpoint.getHostAndPort(), + + try (ConnectionPool pool = new ConnectionPool(endpoint.getHostAndPort(), endpoint.getClientConfigBuilder().authXManager(jedisAuthXManager).build()) { @Override public void evict() throws Exception { numberOfEvictions.incrementAndGet(); super.evict(); } - }; - - await().pollInterval(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) - .until(numberOfEvictions::get, Matchers.greaterThanOrEqualTo(1)); + }) { + await().pollInterval(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) + .until(numberOfEvictions::get, Matchers.greaterThanOrEqualTo(1)); + } } public static class TokenManagerConfigWrapper extends TokenManagerConfig { @@ -227,8 +229,12 @@ public void testAuthXManagerReceivesNewToken() return null; }).when(manager).authenticateConnections(any()); - manager.start(); - assertEquals(tokenHolder[0].getValue(), "tokenVal"); + try { + manager.start(); + assertEquals(tokenHolder[0].getValue(), "tokenVal"); + } finally { + manager.stop(); + } } @Test @@ -292,14 +298,18 @@ public void testTokenManagerWithFailingTokenRequest() TokenManager tokenManager = new TokenManager(identityProvider, new TokenManagerConfig(0.7F, 200, 2000, new TokenManagerConfig.RetryPolicy(numberOfRetries - 1, 100))); - TokenListener listener = mock(TokenListener.class); - tokenManager.start(listener, false); - requesLatch.await(); - await().pollDelay(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) - .untilAsserted(() -> verify(listener).onTokenRenewed(argument.capture())); - verify(identityProvider, times(numberOfRetries)).requestToken(); - verify(listener, never()).onError(any()); - assertEquals("tokenValX", argument.getValue().getValue()); + try { + TokenListener listener = mock(TokenListener.class); + tokenManager.start(listener, false); + requesLatch.await(); + await().pollDelay(ONE_HUNDRED_MILLISECONDS).atMost(FIVE_HUNDRED_MILLISECONDS) + .untilAsserted(() -> verify(listener).onTokenRenewed(argument.capture())); + verify(identityProvider, times(numberOfRetries)).requestToken(); + verify(listener, never()).onError(any()); + assertEquals("tokenValX", argument.getValue().getValue()); + } finally { + tokenManager.stop(); + } } @Test @@ -328,15 +338,19 @@ public void testTokenManagerWithHangingTokenRequest() executionTimeout, new TokenManagerConfig.RetryPolicy(numberOfRetries, 100))); AuthXManager manager = spy(new AuthXManager(tokenManager)); - AuthXEventListener listener = mock(AuthXEventListener.class); - manager.setListener(listener); - manager.start(); - requesLatch.await(); - verify(listener, never()).onIdentityProviderError(any()); - verify(listener, never()).onConnectionAuthenticationError(any()); - - await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> { - verify(manager, times(1)).authenticateConnections(any()); - }); + try { + AuthXEventListener listener = mock(AuthXEventListener.class); + manager.setListener(listener); + manager.start(); + requesLatch.await(); + verify(listener, never()).onIdentityProviderError(any()); + verify(listener, never()).onConnectionAuthenticationError(any()); + + await().atMost(2, TimeUnit.SECONDS).untilAsserted(() -> { + verify(manager, times(1)).authenticateConnections(any()); + }); + } finally { + manager.stop(); + } } }