This is an automated email from the ASF dual-hosted git repository. frankgh pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push: new af0060f CASSANDRASC-113 Fix flaky JmxClientTest (#105) af0060f is described below commit af0060f1325bf8edf74171f60326a3427e13e01d Author: Francisco Guerrero <fran...@apache.org> AuthorDate: Thu Mar 7 16:25:18 2024 -0800 CASSANDRASC-113 Fix flaky JmxClientTest (#105) In this PR, we fix the race condition that occurs when determining the port number to use for the registry. Currently, the port is determined in the `availablePort` method, where a socket is determined by using port 0. The OS will assign a port number for the socket, but we immediately close the socket, and use the determined port number to run the test. This PR brings a better approach by directly using port 0 while creating the registry, thus avoiding the intermediate step and directly using the port that originally was assigned to the registry without releasing it until the end of the test. Additionally in this PR, we rename the integration test JmxClientTest which name is colliding with the unit test. This allows for a better IDE integration and debugging experience. Patch by Francisco Guerrero; Reviewed by Yifan Cai for CASSANDRASC-113 --- .../cassandra/sidecar/common/JmxClientTest.java | 68 ++++++++++------------ ...ientTest.java => JmxClientIntegrationTest.java} | 2 +- 2 files changed, 33 insertions(+), 37 deletions(-) diff --git a/common/src/test/java/org/apache/cassandra/sidecar/common/JmxClientTest.java b/common/src/test/java/org/apache/cassandra/sidecar/common/JmxClientTest.java index 0744bbc..a3cfe86 100644 --- a/common/src/test/java/org/apache/cassandra/sidecar/common/JmxClientTest.java +++ b/common/src/test/java/org/apache/cassandra/sidecar/common/JmxClientTest.java @@ -20,11 +20,11 @@ package org.apache.cassandra.sidecar.common; import java.io.IOException; import java.lang.management.ManagementFactory; -import java.net.MalformedURLException; -import java.net.ServerSocket; import java.nio.file.Path; import java.rmi.registry.LocateRegistry; import java.rmi.registry.Registry; +import java.rmi.server.RemoteObject; +import java.rmi.server.RemoteRef; import java.rmi.server.UnicastRemoteObject; import java.util.Arrays; import java.util.Collections; @@ -52,6 +52,8 @@ import org.junit.jupiter.api.io.TempDir; import org.junit.platform.commons.util.Preconditions; import org.apache.cassandra.sidecar.common.exceptions.JmxAuthenticationException; +import sun.rmi.server.UnicastRef; +import sun.rmi.transport.LiveRef; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -65,12 +67,12 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * to make JMX calls. This particular call happens to match the signature of the `importNewSSTables` method on * StorageServiceProxy in C* 4.0. */ -public class JmxClientTest +class JmxClientTest { - private static final int port; - private static final JMXServiceURL serviceURL; private static final String objectName = "org.apache.cassandra.jmx:type=ExtendedImport"; public static final int PROXIES_TO_TEST = 10_000; + private static int port; + private static JMXServiceURL serviceURL; private static StorageService importMBean; private static JMXConnectorServer jmxServer; private static MBeanServer mbs; @@ -90,8 +92,13 @@ public class JmxClientTest .toString(); Map<String, String> env = new HashMap<>(); env.put("jmx.remote.x.password.file", passwordFile); - registry = LocateRegistry.createRegistry(port); + registry = LocateRegistry.createRegistry(0); // dynamically allocate a port number mbs = ManagementFactory.getPlatformMBeanServer(); + + port = determinePortNumber(registry); + + serviceURL = new JMXServiceURL("service:jmx:rmi://127.0.0.1:" + port + + "/jndi/rmi://127.0.0.1:" + port + "/jmxrmi"); jmxServer = JMXConnectorServerFactory.newJMXConnectorServer(serviceURL, env, mbs); jmxServer.start(); importMBean = new StorageService(); @@ -118,7 +125,7 @@ public class JmxClientTest } @Test - public void testCanCallMethodWithoutEntireInterface() throws IOException + void testCanCallMethodWithoutEntireInterface() throws IOException { List<String> result; try (JmxClient client = JmxClient.builder() @@ -136,7 +143,7 @@ public class JmxClientTest } @Test - public void testCanCallMethodWithoutEntireInterfaceGetResults() throws IOException + void testCanCallMethodWithoutEntireInterfaceGetResults() throws IOException { importMBean.shouldSucceed = false; HashSet<String> srcPaths; @@ -158,7 +165,7 @@ public class JmxClientTest } @Test - public void testCallWithoutCredentialsFails() throws IOException + void testCallWithoutCredentialsFails() throws IOException { try (JmxClient client = JmxClient.builder().jmxServiceURL(serviceURL).build()) { @@ -177,7 +184,7 @@ public class JmxClientTest } @Test - public void testRoleSupplierThrows() throws IOException + void testRoleSupplierThrows() throws IOException { String errorMessage = "bad role state!"; Supplier<String> roleSupplier = () -> { @@ -190,7 +197,7 @@ public class JmxClientTest } @Test - public void testPasswordSupplierThrows() throws IOException + void testPasswordSupplierThrows() throws IOException { String errorMessage = "bad password state!"; Supplier<String> passwordSupplier = () -> { @@ -204,7 +211,7 @@ public class JmxClientTest } @Test - public void testEnableSslSupplierThrows() throws IOException + void testEnableSslSupplierThrows() throws IOException { String errorMessage = "bad ssl supplier state!"; BooleanSupplier enableSslSupplier = () -> { @@ -219,7 +226,7 @@ public class JmxClientTest } @Test - public void testRetryAfterAuthenticationFailureWithCorrectCredentials() throws IOException + void testRetryAfterAuthenticationFailureWithCorrectCredentials() throws IOException { AtomicInteger tryCount = new AtomicInteger(0); List<String> result; @@ -260,7 +267,7 @@ public class JmxClientTest } @Test - public void testDisconnectReconnect() throws Exception + void testDisconnectReconnect() throws Exception { List<String> result; try (JmxClient client = JmxClient.builder() @@ -291,7 +298,7 @@ public class JmxClientTest } @Test - public void testLotsOfProxies() throws IOException + void testLotsOfProxies() throws IOException { try (JmxClient client = JmxClient.builder() .jmxServiceURL(serviceURL) @@ -312,7 +319,7 @@ public class JmxClientTest } @Test - public void testConstructorWithHostPort() throws IOException + void testConstructorWithHostPort() throws IOException { try (JmxClient client = JmxClient.builder() .host("127.0.0.1") @@ -398,29 +405,18 @@ public class JmxClientTest } } - static + static int determinePortNumber(Registry registry) { - try - { - port = availablePort(); - serviceURL = new JMXServiceURL("service:jmx:rmi://127.0.0.1:" + port - + "/jndi/rmi://127.0.0.1:" + port + "/jmxrmi"); - } - catch (MalformedURLException e) + if (registry instanceof RemoteObject) { - throw new RuntimeException(e); - } - } + RemoteRef ref = ((RemoteObject) registry).getRef(); - private static int availablePort() - { - try (ServerSocket socket = new ServerSocket(0)) - { - return socket.getLocalPort(); - } - catch (IOException exception) - { - return 9999; + if (ref instanceof UnicastRef) + { + LiveRef liveRef = ((UnicastRef) ref).getLiveRef(); + return liveRef.getPort(); + } } + throw new RuntimeException("Unable to determine port number"); } } diff --git a/src/test/integration/org/apache/cassandra/sidecar/common/JmxClientTest.java b/src/test/integration/org/apache/cassandra/sidecar/common/JmxClientIntegrationTest.java similarity index 99% rename from src/test/integration/org/apache/cassandra/sidecar/common/JmxClientTest.java rename to src/test/integration/org/apache/cassandra/sidecar/common/JmxClientIntegrationTest.java index e152850..38578fd 100644 --- a/src/test/integration/org/apache/cassandra/sidecar/common/JmxClientTest.java +++ b/src/test/integration/org/apache/cassandra/sidecar/common/JmxClientIntegrationTest.java @@ -32,7 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * Test to ensure connectivity with the JMX client */ -public class JmxClientTest +public class JmxClientIntegrationTest { private static final String SS_OBJ_NAME = "org.apache.cassandra.db:type=StorageService"; --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org