This is an automated email from the ASF dual-hosted git repository.
agoncharuk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new ea97f8a IGNITE-13753 Fix non-thread-safe collection in
JmxMetricExporterSpi - Fixes #8492.
ea97f8a is described below
commit ea97f8ab24a74eac75b41e41bfa33ada1ee269f4
Author: Alexey Goncharuk <alexey.goncha...@gmail.com>
AuthorDate: Thu Nov 26 19:22:09 2020 +0300
IGNITE-13753 Fix non-thread-safe collection in JmxMetricExporterSpi - Fixes
#8492.
Signed-off-by: Alexey Goncharuk <alexey.goncha...@gmail.com>
---
.../spi/metric/jmx/JmxMetricExporterSpi.java | 9 +-
.../ignite/spi/metric/jmx/DummyMBeanServer.java | 291 +++++++++++++++++++++
.../spi/metric/jmx/JmxMetricExporterSpiTest.java | 141 ++++++++++
.../ignite/testsuites/IgniteSpiTestSuite.java | 5 +-
4 files changed, 443 insertions(+), 3 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpi.java
b/modules/core/src/main/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpi.java
index 7671a81..fe56007 100644
---
a/modules/core/src/main/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpi.java
+++
b/modules/core/src/main/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpi.java
@@ -18,6 +18,7 @@
package org.apache.ignite.spi.metric.jmx;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.function.Predicate;
import javax.management.JMException;
@@ -46,7 +47,7 @@ public class JmxMetricExporterSpi extends IgniteSpiAdapter
implements MetricExpo
private @Nullable Predicate<ReadOnlyMetricRegistry> filter;
/** Registered beans. */
- private final List<ObjectName> mBeans = new ArrayList<>();
+ private final List<ObjectName> mBeans = Collections.synchronizedList(new
ArrayList<>());
/** {@inheritDoc} */
@Override public void spiStart(@Nullable String igniteInstanceName) throws
IgniteSpiException {
@@ -127,6 +128,10 @@ public class JmxMetricExporterSpi extends IgniteSpiAdapter
implements MetricExpo
unregBean(ignite, bean);
}
+ /**
+ * @param ignite Ignite instance.
+ * @param bean Bean name to unregister.
+ */
private void unregBean(Ignite ignite, ObjectName bean) {
MBeanServer jmx = ignite.configuration().getMBeanServer();
@@ -143,7 +148,7 @@ public class JmxMetricExporterSpi extends IgniteSpiAdapter
implements MetricExpo
/** {@inheritDoc} */
@Override public void setMetricRegistry(ReadOnlyMetricManager reg) {
- this.mreg = reg;
+ mreg = reg;
}
/** {@inheritDoc} */
diff --git
a/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/DummyMBeanServer.java
b/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/DummyMBeanServer.java
new file mode 100644
index 0000000..4d9c467
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/DummyMBeanServer.java
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.spi.metric.jmx;
+
+import java.io.ObjectInputStream;
+import java.util.Set;
+import javax.management.Attribute;
+import javax.management.AttributeList;
+import javax.management.MBeanInfo;
+import javax.management.MBeanServer;
+import javax.management.NotificationFilter;
+import javax.management.NotificationListener;
+import javax.management.ObjectInstance;
+import javax.management.ObjectName;
+import javax.management.QueryExp;
+import javax.management.loading.ClassLoaderRepository;
+
+/**
+ *
+ */
+class DummyMBeanServer implements MBeanServer {
+ /** */
+ public static final String[] DOMAINS = new String[0];
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance createMBean(String clsName, ObjectName
name) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance createMBean(String clsName, ObjectName
name, ObjectName ldrName) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance createMBean(String clsName, ObjectName
name, Object[] params, String[] signature) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance createMBean(String clsName, ObjectName
name, ObjectName ldrName, Object[] params, String[] signature) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance registerMBean(Object obj, ObjectName name)
{
+ return new ObjectInstance(name, obj.getClass().getName());
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void unregisterMBean(ObjectName name) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInstance getObjectInstance(ObjectName name) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Set<ObjectInstance> queryMBeans(ObjectName name, QueryExp
qry) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Set<ObjectName> queryNames(ObjectName name, QueryExp qry)
{
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean isRegistered(ObjectName name) {
+ return false;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Integer getMBeanCount() {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object getAttribute(ObjectName name, String attribute) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public AttributeList getAttributes(ObjectName name, String[]
atts) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void setAttribute(ObjectName name, Attribute attribute) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public AttributeList setAttributes(ObjectName name,
AttributeList atts) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object invoke(ObjectName name, String operationName,
Object[] params, String[] signature) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public String getDefaultDomain() {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public String[] getDomains() {
+ return DOMAINS;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void addNotificationListener(ObjectName name,
NotificationListener lsnr, NotificationFilter filter, Object handback) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void addNotificationListener(ObjectName name, ObjectName
lsnr, NotificationFilter filter, Object handback) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void removeNotificationListener(ObjectName name,
ObjectName lsnr) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void removeNotificationListener(ObjectName name,
ObjectName lsnr, NotificationFilter filter, Object handback) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void removeNotificationListener(ObjectName name,
NotificationListener lsnr) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public void removeNotificationListener(ObjectName name,
NotificationListener lsnr, NotificationFilter filter, Object handback) {
+
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public MBeanInfo getMBeanInfo(ObjectName name) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean isInstanceOf(ObjectName name, String clsName) {
+ return false;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object instantiate(String clsName) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object instantiate(String clsName, ObjectName ldrName) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object instantiate(String clsName, Object[] params,
String[] signature) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public Object instantiate(String clsName, ObjectName ldrName,
Object[] params, String[] signature) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInputStream deserialize(ObjectName name, byte[]
data) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInputStream deserialize(String clsName, byte[]
data) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ObjectInputStream deserialize(String clsName, ObjectName
ldrName, byte[] data) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ClassLoader getClassLoaderFor(ObjectName mbeanName) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ClassLoader getClassLoader(ObjectName ldrName) {
+ return null;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override public ClassLoaderRepository getClassLoaderRepository() {
+ return null;
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpiTest.java
b/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpiTest.java
new file mode 100644
index 0000000..ee6e0f9
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpiTest.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.spi.metric.jmx;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import org.apache.commons.collections.iterators.EmptyIterator;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricManager;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.IgniteTestResources;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class JmxMetricExporterSpiTest extends GridCommonAbstractTest {
+ /**
+ *
+ */
+ @Test
+ public void testConcurrentRegistration() throws IgniteCheckedException {
+ JmxMetricExporterSpi spi = new JmxMetricExporterSpi();
+
+ new IgniteTestResources(new DummyMBeanServer()).inject(spi);
+
+ TestMetricsManager testMgr = new TestMetricsManager();
+
+ spi.setMetricRegistry(testMgr);
+
+ spi.spiStart("testInstance");
+
+ testMgr.runRegistersConcurrent();
+ testMgr.runUnregisters();
+ }
+
+ /**
+ *
+ */
+ @SuppressWarnings("unchecked")
+ private static class TestMetricsManager implements ReadOnlyMetricManager {
+ /** */
+ private final List<Consumer<ReadOnlyMetricRegistry>> creation = new
ArrayList<>();
+
+ /** */
+ private final List<Consumer<ReadOnlyMetricRegistry>> rmv = new
ArrayList<>();
+
+ /** {@inheritDoc} */
+ @Override public void
addMetricRegistryCreationListener(Consumer<ReadOnlyMetricRegistry> lsnr) {
+ creation.add(lsnr);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void
addMetricRegistryRemoveListener(Consumer<ReadOnlyMetricRegistry> lsnr) {
+ rmv.add(lsnr);
+ }
+
+ /** {@inheritDoc} */
+ @NotNull @Override public Iterator<ReadOnlyMetricRegistry> iterator() {
+ return EmptyIterator.INSTANCE;
+ }
+
+ /**
+ *
+ */
+ public void runRegistersConcurrent() {
+ final AtomicInteger cntr = new AtomicInteger();
+
+ GridTestUtils.runMultiThreadedAsync(() -> {
+ for (int i = 0; i < 20; i++) {
+ for (Consumer<ReadOnlyMetricRegistry> lsnr : creation)
+ lsnr.accept(new ReadOnlyMetricRegistryStub("stub-" +
cntr.getAndIncrement()));
+ }
+ }, Runtime.getRuntime().availableProcessors() * 2, "runner-");
+
+ }
+
+ /**
+ *
+ */
+ public void runUnregisters() {
+ for (int i = 0; i < Runtime.getRuntime().availableProcessors() * 2
* 20; i++) {
+ for (Consumer<ReadOnlyMetricRegistry> lsnr : creation)
+ lsnr.accept(new ReadOnlyMetricRegistryStub("stub-" + i));
+ }
+ }
+
+ /**
+ *
+ */
+ private static class ReadOnlyMetricRegistryStub implements
ReadOnlyMetricRegistry {
+ /** */
+ private final String name;
+
+ /**
+ * @param name Stub name.
+ */
+ private ReadOnlyMetricRegistryStub(String name) {
+ this.name = name;
+ }
+
+ /** {@inheritDoc} */
+ @Override public String name() {
+ return name;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <M extends Metric> @Nullable M findMetric(String
name) {
+ return null;
+ }
+
+ /** {@inheritDoc} */
+ @NotNull @Override public Iterator<Metric> iterator() {
+ return EmptyIterator.INSTANCE;
+ }
+ }
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteSpiTestSuite.java
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteSpiTestSuite.java
index 8a7f0da..bd53f92 100644
---
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteSpiTestSuite.java
+++
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteSpiTestSuite.java
@@ -20,6 +20,7 @@ package org.apache.ignite.testsuites;
import
org.apache.ignite.internal.managers.GridManagerLocalMessageListenerSelfTest;
import org.apache.ignite.internal.managers.GridNoopManagerSelfTest;
import org.apache.ignite.spi.encryption.KeystoreEncryptionSpiSelfTest;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpiTest;
import org.junit.runner.RunWith;
import org.junit.runners.Suite;
@@ -58,7 +59,9 @@ import org.junit.runners.Suite;
// Local Message Listener tests.
GridManagerLocalMessageListenerSelfTest.class,
- KeystoreEncryptionSpiSelfTest.class
+ KeystoreEncryptionSpiSelfTest.class,
+
+ JmxMetricExporterSpiTest.class
})
public class IgniteSpiTestSuite {
}
