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>
redis · Jan 15, 2025 · 27ecd6c · 27ecd6c
1 parent f9977e4
commit 27ecd6c
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 30 deletions.
diff --git 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
@@ -234,6 +234,7 @@
 				<artifactId>maven-surefire-plugin</artifactId>
 				<version>${maven.surefire.version}</version>
 				<configuration>
+					<argLine>${JVM_OPTS}</argLine>
 					<systemPropertyVariables>
 						<redis-hosts>${redis-hosts}</redis-hosts>
 					</systemPropertyVariables>

diff --git 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();
+    }
   }
 }