From 2c48bab91a5bd067f68a8a19da44c964553ff9b3 Mon Sep 17 00:00:00 2001 From: Piotr Jastrzebski Date: Sat, 12 May 2018 18:35:18 +0200 Subject: [PATCH] Use JmxMBeanServer instead of MBeanServer JmxMBeanServer is a concrete implementation of a MBeanServer. We want to use it directly because we need to bypass calls to JmxMBeanServer.registerMBean and JmxMBeanServer.unregisterMBean. They take ObjectName as parameter, copy it using ObjectName.getInstance(ObjectName) and pass it to registerMBean and unregisterMBean of JmxMBeanServer.getMBeanServerInterceptor(). We want to avoid this copy and pass the ObjectName argument directly to JmxMBeanServer.getMBeanServerInterceptor(). To do that this patch: 1. changes all MBeanServer variables to JmxMBeanServer 2. creates JmxMBeanServer in APIBuilder making sure accessing interceptors is allowed 3. makes sure that JmxMBeanServer.getMBeanServerInterceptor().registerMBean is called wherever JmxMBeanServer.registerMBean was called 4. makes sure that JmxMBeanServer.getMBeanServerInterceptor().unregisterMBean is called whenever JmxMBeanServer.unregisterMBean was called Next patch will use different ObjectName implementation that will use less memory and this patch is crucial because without it every ObjectName is transformed with ObjectName.getInstance which turns the object into a regular ObjectName. Signed-off-by: Piotr Jastrzebski --- .../java/com/scylladb/jmx/metrics/APIMBean.java | 13 +++++++------ .../java/com/scylladb/jmx/metrics/MetricsMBean.java | 7 ++++--- .../java/com/scylladb/jmx/utils/APIBuilder.java | 6 +++++- .../java/com/scylladb/jmx/utils/APIMBeanServer.java | 5 +++-- .../org/apache/cassandra/db/ColumnFamilyStore.java | 3 ++- .../apache/cassandra/metrics/MetricsRegistry.java | 7 ++++--- .../apache/cassandra/metrics/StreamingMetrics.java | 6 +++--- 7 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/scylladb/jmx/metrics/APIMBean.java b/src/main/java/com/scylladb/jmx/metrics/APIMBean.java index 4c57171..8eb8a01 100644 --- a/src/main/java/com/scylladb/jmx/metrics/APIMBean.java +++ b/src/main/java/com/scylladb/jmx/metrics/APIMBean.java @@ -20,6 +20,7 @@ import javax.management.ObjectName; import javax.management.QueryExp; import com.scylladb.jmx.api.APIClient; +import com.sun.jmx.mbeanserver.JmxMBeanServer; /** * Base type for MBeans in scylla-jmx. Wraps auto naming and {@link APIClient} @@ -58,14 +59,14 @@ public class APIMBean implements MBeanRegistration { * @return * @throws MalformedObjectNameException */ - public static boolean checkRegistration(MBeanServer server, Set all, + public static boolean checkRegistration(JmxMBeanServer server, Set all, final Predicate predicate, Function generator) throws MalformedObjectNameException { Set registered = queryNames(server, predicate); for (ObjectName name : registered) { if (!all.contains(name)) { try { - server.unregisterMBean(name); + server.getMBeanServerInterceptor().unregisterMBean(name); } catch (MBeanRegistrationException | InstanceNotFoundException e) { } } @@ -75,7 +76,7 @@ public class APIMBean implements MBeanRegistration { for (ObjectName name : all) { if (!registered.contains(name)) { try { - server.registerMBean(generator.apply(name), name); + server.getMBeanServerInterceptor().registerMBean(generator.apply(name), name); added++; } catch (InstanceAlreadyExistsException | MBeanRegistrationException | NotCompliantMBeanException e) { } @@ -92,7 +93,7 @@ public class APIMBean implements MBeanRegistration { * @param predicate * @return */ - public static Set queryNames(MBeanServer server, final Predicate predicate) { + public static Set queryNames(JmxMBeanServer server, final Predicate predicate) { @SuppressWarnings("serial") Set registered = server.queryNames(null, new QueryExp() { @Override @@ -108,7 +109,7 @@ public class APIMBean implements MBeanRegistration { return registered; } - MBeanServer server; + JmxMBeanServer server; ObjectName name; protected final ObjectName getBoundName() { @@ -162,7 +163,7 @@ public class APIMBean implements MBeanRegistration { if (this.server != null) { throw new IllegalStateException("Can only exist in a single MBeanServer"); } - this.server = server; + this.server = (JmxMBeanServer) server; if (name == null) { name = generateName(); } diff --git a/src/main/java/com/scylladb/jmx/metrics/MetricsMBean.java b/src/main/java/com/scylladb/jmx/metrics/MetricsMBean.java index ae4a87f..90aada6 100644 --- a/src/main/java/com/scylladb/jmx/metrics/MetricsMBean.java +++ b/src/main/java/com/scylladb/jmx/metrics/MetricsMBean.java @@ -16,6 +16,7 @@ import org.apache.cassandra.metrics.Metrics; import org.apache.cassandra.metrics.MetricsRegistry; import com.scylladb.jmx.api.APIClient; +import com.sun.jmx.mbeanserver.JmxMBeanServer; /** * Base type for MBeans containing {@link Metrics}. @@ -47,7 +48,7 @@ public abstract class MetricsMBean extends APIMBean { }; } - private void register(MetricsRegistry registry, MBeanServer server) throws MalformedObjectNameException { + private void register(MetricsRegistry registry, JmxMBeanServer server) throws MalformedObjectNameException { // Check if we're the first/last of our type bound/removed. boolean empty = queryNames(server, getTypePredicate()).isEmpty(); for (Metrics m : metrics) { @@ -63,7 +64,7 @@ public abstract class MetricsMBean extends APIMBean { // Get name etc. name = super.preRegister(server, name); // Register all metrics in server - register(new MetricsRegistry(client, server), server); + register(new MetricsRegistry(client, (JmxMBeanServer) server), (JmxMBeanServer) server); return name; } @@ -77,7 +78,7 @@ public abstract class MetricsMBean extends APIMBean { public void register(Supplier s, ObjectName... objectNames) { for (ObjectName name : objectNames) { try { - server.unregisterMBean(name); + server.getMBeanServerInterceptor().unregisterMBean(name); } catch (MBeanRegistrationException | InstanceNotFoundException e) { } } diff --git a/src/main/java/com/scylladb/jmx/utils/APIBuilder.java b/src/main/java/com/scylladb/jmx/utils/APIBuilder.java index b7fb775..cc16ac4 100644 --- a/src/main/java/com/scylladb/jmx/utils/APIBuilder.java +++ b/src/main/java/com/scylladb/jmx/utils/APIBuilder.java @@ -26,10 +26,14 @@ import javax.management.MBeanServer; import javax.management.MBeanServerBuilder; import javax.management.MBeanServerDelegate; +import com.sun.jmx.mbeanserver.JmxMBeanServer; + public class APIBuilder extends MBeanServerBuilder { @Override public MBeanServer newMBeanServer(String defaultDomain, MBeanServer outer, MBeanServerDelegate delegate) { - MBeanServer nested = super.newMBeanServer(defaultDomain, outer, delegate); + // It is important to set |interceptors| to true while creating the JmxMBeanSearver. + // It is required for calls to JmxMBeanServer.getMBeanServerInterceptor() to be allowed. + JmxMBeanServer nested = (JmxMBeanServer) JmxMBeanServer.newMBeanServer(defaultDomain, outer, delegate, true); return new APIMBeanServer(client, nested); } } \ No newline at end of file diff --git a/src/main/java/com/scylladb/jmx/utils/APIMBeanServer.java b/src/main/java/com/scylladb/jmx/utils/APIMBeanServer.java index 6f2f8f4..9d5ae86 100644 --- a/src/main/java/com/scylladb/jmx/utils/APIMBeanServer.java +++ b/src/main/java/com/scylladb/jmx/utils/APIMBeanServer.java @@ -33,15 +33,16 @@ import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.metrics.StreamingMetrics; import com.scylladb.jmx.api.APIClient; +import com.sun.jmx.mbeanserver.JmxMBeanServer; public class APIMBeanServer implements MBeanServer { @SuppressWarnings("unused") private static final Logger logger = Logger.getLogger(APIMBeanServer.class.getName()); private final APIClient client; - private final MBeanServer server; + private final JmxMBeanServer server; - public APIMBeanServer(APIClient client, MBeanServer server) { + public APIMBeanServer(APIClient client, JmxMBeanServer server) { this.client = client; this.server = server; } diff --git a/src/main/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/main/java/org/apache/cassandra/db/ColumnFamilyStore.java index ea62b14..8cc7bea 100644 --- a/src/main/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/main/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -54,6 +54,7 @@ import org.apache.cassandra.metrics.TableMetrics; import com.scylladb.jmx.api.APIClient; import com.scylladb.jmx.metrics.MetricsMBean; +import com.sun.jmx.mbeanserver.JmxMBeanServer; public class ColumnFamilyStore extends MetricsMBean implements ColumnFamilyStoreMBean { private static final Logger logger = Logger.getLogger(ColumnFamilyStore.class.getName()); @@ -102,7 +103,7 @@ public class ColumnFamilyStore extends MetricsMBean implements ColumnFamilyStore "org.apache.cassandra.db:type=" + type + ",keyspace=" + keyspace + ",columnfamily=" + name); } - public static boolean checkRegistration(APIClient client, MBeanServer server) throws MalformedObjectNameException { + public static boolean checkRegistration(APIClient client, JmxMBeanServer server) throws MalformedObjectNameException { JsonArray mbeans = client.getJsonArray("/column_family/"); Set all = new HashSet(); for (int i = 0; i < mbeans.size(); i++) { diff --git a/src/main/java/org/apache/cassandra/metrics/MetricsRegistry.java b/src/main/java/org/apache/cassandra/metrics/MetricsRegistry.java index eee2c30..a17cf80 100644 --- a/src/main/java/org/apache/cassandra/metrics/MetricsRegistry.java +++ b/src/main/java/org/apache/cassandra/metrics/MetricsRegistry.java @@ -39,6 +39,7 @@ import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; import com.scylladb.jmx.api.APIClient; +import com.sun.jmx.mbeanserver.JmxMBeanServer; /** * Makes integrating 3.0 metrics API with 2.0. @@ -53,9 +54,9 @@ public class MetricsRegistry { private static final Logger logger = Logger.getLogger(MetricsRegistry.class.getName()); private final APIClient client; - private final MBeanServer mBeanServer; + private final JmxMBeanServer mBeanServer; - public MetricsRegistry(APIClient client, MBeanServer mBeanServer) { + public MetricsRegistry(APIClient client, JmxMBeanServer mBeanServer) { this.client = client; this.mBeanServer = mBeanServer; } @@ -108,7 +109,7 @@ public class MetricsRegistry { MetricMBean bean = f.get(); for (ObjectName name : objectNames) { try { - mBeanServer.registerMBean(bean, name); + mBeanServer.getMBeanServerInterceptor().registerMBean(bean, name); } catch (InstanceAlreadyExistsException | MBeanRegistrationException | NotCompliantMBeanException e) { logger.log(SEVERE, "Could not register mbean", e); } diff --git a/src/main/java/org/apache/cassandra/metrics/StreamingMetrics.java b/src/main/java/org/apache/cassandra/metrics/StreamingMetrics.java index b2eb35f..fc4fd17 100644 --- a/src/main/java/org/apache/cassandra/metrics/StreamingMetrics.java +++ b/src/main/java/org/apache/cassandra/metrics/StreamingMetrics.java @@ -33,12 +33,12 @@ import java.util.HashSet; import java.util.Set; import javax.json.JsonArray; -import javax.management.MBeanServer; import javax.management.MalformedObjectNameException; import javax.management.ObjectName; import com.scylladb.jmx.api.APIClient; import com.scylladb.jmx.metrics.APIMBean; +import com.sun.jmx.mbeanserver.JmxMBeanServer; /** * Metrics for streaming. @@ -65,11 +65,11 @@ public class StreamingMetrics { return TYPE_NAME.equals(n.getKeyProperty("type")); } - public static void unregister(APIClient client, MBeanServer server) throws MalformedObjectNameException { + public static void unregister(APIClient client, JmxMBeanServer server) throws MalformedObjectNameException { APIMBean.checkRegistration(server, emptySet(), StreamingMetrics::isStreamingName, (n) -> null); } - public static boolean checkRegistration(APIClient client, MBeanServer server) + public static boolean checkRegistration(APIClient client, JmxMBeanServer server) throws MalformedObjectNameException, UnknownHostException { Set all = new HashSet(globalNames);