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 { }