diff --git a/build.xml b/build.xml
index 3593d4bd70..8d9b8f5516 100644
--- a/build.xml
+++ b/build.xml
@@ -153,7 +153,7 @@
-
+
@@ -218,6 +218,7 @@
--add-exports java.management.rmi/com.sun.jmx.remote.internal.rmi=ALL-UNNAMED--add-exports java.rmi/sun.rmi.registry=ALL-UNNAMED--add-exports java.rmi/sun.rmi.server=ALL-UNNAMED
+ --add-exports java.rmi/sun.rmi.transport=ALL-UNNAMED--add-exports java.rmi/sun.rmi.transport.tcp=ALL-UNNAMED--add-exports java.sql/java.sql=ALL-UNNAMED
@@ -2173,7 +2174,7 @@
-
+
]]>
diff --git a/src/java/org/apache/cassandra/utils/RMIClientSocketFactoryImpl.java b/src/java/org/apache/cassandra/utils/RMIClientSocketFactoryImpl.java
index 62ab88fb92..5caa92cce4 100644
--- a/src/java/org/apache/cassandra/utils/RMIClientSocketFactoryImpl.java
+++ b/src/java/org/apache/cassandra/utils/RMIClientSocketFactoryImpl.java
@@ -23,6 +23,8 @@ import java.io.Serializable;
import java.net.InetAddress;
import java.net.Socket;
import java.rmi.server.RMIClientSocketFactory;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Objects;
/**
@@ -32,6 +34,7 @@ import java.util.Objects;
*/
public class RMIClientSocketFactoryImpl implements RMIClientSocketFactory, Serializable
{
+ List sockets = new ArrayList<>();
private final InetAddress localAddress;
public RMIClientSocketFactoryImpl(InetAddress localAddress)
@@ -42,7 +45,23 @@ public class RMIClientSocketFactoryImpl implements RMIClientSocketFactory, Seria
@Override
public Socket createSocket(String host, int port) throws IOException
{
- return new Socket(localAddress, port);
+ Socket socket = new Socket(localAddress, port);
+ sockets.add(socket);
+ return socket;
+ }
+
+ public void close() throws IOException
+ {
+ for (Socket socket: sockets) {
+ try
+ {
+ socket.close();
+ }
+ catch (IOException ignored)
+ {
+ // intentionally ignored
+ }
+ }
}
@Override
diff --git a/src/java/org/apache/cassandra/utils/ReflectionUtils.java b/src/java/org/apache/cassandra/utils/ReflectionUtils.java
index bd605db3bc..07802267f1 100644
--- a/src/java/org/apache/cassandra/utils/ReflectionUtils.java
+++ b/src/java/org/apache/cassandra/utils/ReflectionUtils.java
@@ -20,6 +20,9 @@ package org.apache.cassandra.utils;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.function.Predicate;
public class ReflectionUtils {
public static Field getField(Class> clazz, String fieldName) throws NoSuchFieldException
@@ -52,4 +55,44 @@ public class ReflectionUtils {
throw e;
}
}
+
+ /**
+ * Used by the in-jvm dtest framework to remove entries from private map fields that otherwise would prevent
+ * collection of classloaders (which causes metaspace OOMs) or otherwise interfere with instance restart.
+ * @param clazz The class which has the map field to clear
+ * @param instance an instance of the class to clear (pass null for a static member)
+ * @param mapName the name of the map field to clear
+ * @param shouldRemove a predicate which determines if the entry in question should be removed
+ * @param The type of the map key
+ * @param The type of the map value
+ */
+ public static void clearMapField(Class> clazz, Object instance, String mapName, Predicate> shouldRemove) {
+ try
+ {
+ Field mapField = getField(clazz, mapName);
+ mapField.setAccessible(true);
+ // noinspection unchecked
+ Map map = (Map) mapField.get(instance);
+ // Because multiple instances can be shutting down at once,
+ // synchronize on the map to avoid ConcurrentModificationException
+ synchronized (map)
+ {
+ // This could be done with a simple `map.entrySet.removeIf()` call
+ // but for debugging purposes it is much easier to keep it like this.
+ Iterator> it = map.entrySet().iterator();
+ while (it.hasNext())
+ {
+ Map.Entry entry = it.next();
+ if (shouldRemove.test(entry))
+ {
+ it.remove();
+ }
+ }
+ }
+ }
+ catch (NoSuchFieldException | IllegalAccessException ex)
+ {
+ throw new RuntimeException(String.format("Could not clear map field %s in class %s", mapName, clazz), ex);
+ }
+ }
}
diff --git a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
index 9975aeb9eb..4b4276774b 100644
--- a/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
+++ b/test/distributed/org/apache/cassandra/distributed/impl/Instance.java
@@ -755,17 +755,18 @@ public class Instance extends IsolatedExecutor implements IInvokableInstance
initialized = true;
}
- private void startJmx()
+ private synchronized void startJmx()
{
this.isolatedJmx = new IsolatedJmx(this, inInstancelogger);
isolatedJmx.startJmx();
}
- private void stopJmx() throws IllegalAccessException, NoSuchFieldException, InterruptedException
+ private synchronized void stopJmx()
{
if (config.has(JMX))
{
isolatedJmx.stopJmx();
+ isolatedJmx = null;
}
}
diff --git a/test/distributed/org/apache/cassandra/distributed/impl/IsolatedJmx.java b/test/distributed/org/apache/cassandra/distributed/impl/IsolatedJmx.java
index e19e29fdb3..2fd4ec9647 100644
--- a/test/distributed/org/apache/cassandra/distributed/impl/IsolatedJmx.java
+++ b/test/distributed/org/apache/cassandra/distributed/impl/IsolatedJmx.java
@@ -18,38 +18,40 @@
package org.apache.cassandra.distributed.impl;
-import java.lang.reflect.Field;
+import java.io.IOException;
import java.net.InetAddress;
-import java.net.MalformedURLException;
import java.util.HashMap;
-import java.util.Iterator;
+import java.util.LinkedList;
import java.util.Map;
+import java.util.concurrent.TimeUnit;
import javax.management.remote.JMXConnector;
-import javax.management.remote.JMXConnectorFactory;
import javax.management.remote.JMXConnectorServer;
import javax.management.remote.JMXServiceURL;
import javax.management.remote.rmi.RMIConnectorServer;
import javax.management.remote.rmi.RMIJRMPServerImpl;
+import com.google.common.util.concurrent.Uninterruptibles;
import org.slf4j.Logger;
import org.apache.cassandra.distributed.api.IInstance;
import org.apache.cassandra.distributed.api.IInstanceConfig;
+import org.apache.cassandra.distributed.shared.JMXUtil;
import org.apache.cassandra.utils.JMXServerUtils;
import org.apache.cassandra.utils.MBeanWrapper;
import org.apache.cassandra.utils.RMIClientSocketFactoryImpl;
-import org.apache.cassandra.utils.ReflectionUtils;
import sun.rmi.transport.tcp.TCPEndpoint;
import static org.apache.cassandra.config.CassandraRelevantProperties.JAVA_RMI_DGC_LEASE_VALUE_IN_JVM_DTEST;
import static org.apache.cassandra.config.CassandraRelevantProperties.ORG_APACHE_CASSANDRA_DISABLE_MBEAN_REGISTRATION;
import static org.apache.cassandra.config.CassandraRelevantProperties.SUN_RMI_TRANSPORT_TCP_THREADKEEPALIVETIME;
import static org.apache.cassandra.distributed.api.Feature.JMX;
+import static org.apache.cassandra.utils.ReflectionUtils.clearMapField;
public class IsolatedJmx
{
- private static final int RMI_KEEPALIVE_TIME = 1000;
+ public static final int RMI_KEEPALIVE_TIME = 1000;
+ public static final String UNKNOWN_JMX_CONNECTION_ERROR = "Could not connect to JMX due to an unknown error";
private JMXConnectorServer jmxConnectorServer;
private JMXServerUtils.JmxRegistry registry;
@@ -127,7 +129,7 @@ public class IsolatedJmx
registry.setRemoteServerStub(jmxRmiServer.toStub());
JMXServerUtils.logJmxServiceUrl(addr, jmxPort);
- waitForJmxAvailability(hostname, jmxPort, env);
+ waitForJmxAvailability(env);
}
catch (Throwable t)
{
@@ -135,36 +137,20 @@ public class IsolatedJmx
}
}
- private void waitForJmxAvailability(String hostname, int rmiPort, Map env) throws InterruptedException, MalformedURLException
+ private void waitForJmxAvailability(Map env)
{
- String url = String.format("service:jmx:rmi:///jndi/rmi://%s:%d/jmxrmi", hostname, rmiPort);
- JMXServiceURL serviceURL = new JMXServiceURL(url);
- int attempts = 0;
- Throwable lastThrown = null;
- while (attempts < 20)
- {
- attempts++;
- try (JMXConnector ignored = JMXConnectorFactory.connect(serviceURL, env))
- {
- inInstancelogger.info("Connected to JMX server at {} after {} attempt(s)",
- url, attempts);
- return;
- }
- catch (MalformedURLException e)
- {
- throw new RuntimeException(e);
- }
- catch (Throwable thrown)
- {
- lastThrown = thrown;
- }
- inInstancelogger.info("Could not connect to JMX on {} after {} attempts. Will retry.", url, attempts);
- Thread.sleep(1000);
+ try (JMXConnector ignored = JMXUtil.getJmxConnector(config, 20, env)) {
+ // Do nothing - JMXUtil now retries
+ }
+ catch (IOException iex)
+ {
+ // If we land here, there's something more than a timeout
+ inInstancelogger.error(UNKNOWN_JMX_CONNECTION_ERROR, iex);
+ throw new RuntimeException(UNKNOWN_JMX_CONNECTION_ERROR, iex);
}
- throw new RuntimeException("Could not start JMX - unreachable after 20 attempts", lastThrown);
}
- public void stopJmx() throws IllegalAccessException, NoSuchFieldException, InterruptedException
+ public void stopJmx()
{
if (!config.has(JMX))
return;
@@ -197,6 +183,14 @@ public class IsolatedJmx
inInstancelogger.warn("failed to close registry.", e);
}
try
+ {
+ clientSocketFactory.close();
+ }
+ catch (Throwable e)
+ {
+ inInstancelogger.warn("failed to close clientSocketFactory.", e);
+ }
+ try
{
serverSocketFactory.close();
}
@@ -206,25 +200,18 @@ public class IsolatedJmx
}
// The TCPEndpoint class holds references to a class in the in-jvm dtest framework
// which transitively has a reference to the InstanceClassLoader, so we need to
- // make sure to remove the reference to them when the instance is shutting down
- clearMapField(TCPEndpoint.class, null, "localEndpoints");
- Thread.sleep(2 * RMI_KEEPALIVE_TIME); // Double the keep-alive time to give Distributed GC some time to clean up
+ // make sure to remove the reference to them when the instance is shutting down.
+ // Additionally, we must make sure to only clear endpoints created by this instance
+ // As clearning the entire map can cause issues with starting and stopping nodes mid-test.
+ clearMapField(TCPEndpoint.class, null, "localEndpoints", this::endpointCreateByThisInstance);
+ Uninterruptibles.sleepUninterruptibly(2 * RMI_KEEPALIVE_TIME, TimeUnit.MILLISECONDS); // Double the keep-alive time to give Distributed GC some time to clean up
}
- private void clearMapField(Class> clazz, Object instance, String mapName)
- throws IllegalAccessException, NoSuchFieldException {
- Field mapField = ReflectionUtils.getField(clazz, mapName);
- mapField.setAccessible(true);
- Map map = (Map) mapField.get(instance);
- // Because multiple instances can be shutting down at once,
- // synchronize on the map to avoid ConcurrentModificationException
- synchronized (map)
- {
- for (Iterator> it = map.entrySet().iterator(); it.hasNext(); )
- {
- it.next();
- it.remove();
- }
- }
+ private boolean endpointCreateByThisInstance(Map.Entry