This is an automated email from the ASF dual-hosted git repository.
sk0x50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 92b0c32404b IGNITE-27296 Fix table metrics registration on recovery
(#7208)
92b0c32404b is described below
commit 92b0c32404bb988d78fdedbb054cd807121ff5d0
Author: Slava Koptilin <[email protected]>
AuthorDate: Mon Dec 15 16:36:48 2025 +0200
IGNITE-27296 Fix table metrics registration on recovery (#7208)
---
.../ignite/internal/metrics/MetricManager.java | 8 +-
.../ignite/internal/metrics/MetricManagerImpl.java | 10 +-
.../metrics/exporters/jmx/MetricSetMbean.java | 15 +--
.../ignite/internal/metrics/MetricManagerTest.java | 113 +++++++++++++++++++++
.../internal/table/distributed/TableManager.java | 29 +++---
5 files changed, 150 insertions(+), 25 deletions(-)
diff --git
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManager.java
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManager.java
index 23212e5bb2f..b9201d06107 100644
---
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManager.java
+++
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManager.java
@@ -65,16 +65,20 @@ public interface MetricManager extends IgniteComponent {
void registerSource(MetricSource src);
/**
- * Unregister metric source. See {@link
MetricRegistry#unregisterSource(MetricSource)}.
+ * Disables and unregisters metric source.
*
* @param src Metric source.
+ * @see #disable(MetricSource)
+ * @see MetricRegistry#unregisterSource(MetricSource)
*/
void unregisterSource(MetricSource src);
/**
- * Unregister metric source by name. See {@link
MetricRegistry#unregisterSource(String)}.
+ * Disables and unregisters metric source by name.
*
* @param srcName Metric source name.
+ * @see #disable(String)
+ * @see MetricRegistry#unregisterSource(String)
*/
void unregisterSource(String srcName);
diff --git
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManagerImpl.java
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManagerImpl.java
index 7232da5a606..7fb5bda92b2 100644
---
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManagerImpl.java
+++
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/MetricManagerImpl.java
@@ -186,12 +186,18 @@ public class MetricManagerImpl implements MetricManager {
@Override
public void unregisterSource(MetricSource src) {
- inBusyLockSafe(busyLock, () -> registry.unregisterSource(src));
+ inBusyLockSafe(busyLock, () -> {
+ disable(src);
+ registry.unregisterSource(src);
+ });
}
@Override
public void unregisterSource(String srcName) {
- inBusyLockSafe(busyLock, () -> registry.unregisterSource(srcName));
+ inBusyLockSafe(busyLock, () -> {
+ disable(srcName);
+ registry.unregisterSource(srcName);
+ });
}
@Override
diff --git
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/jmx/MetricSetMbean.java
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/jmx/MetricSetMbean.java
index c220233d64d..e6072ce47fe 100644
---
a/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/jmx/MetricSetMbean.java
+++
b/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/jmx/MetricSetMbean.java
@@ -45,7 +45,7 @@ import org.apache.ignite.internal.metrics.UuidGauge;
*/
public class MetricSetMbean implements DynamicMBean {
/** Metric set. */
- private MetricSet metricSet;
+ private final MetricSet metricSet;
/**
* Constructs new MBean.
@@ -63,6 +63,9 @@ public class MetricSetMbean implements DynamicMBean {
}
Metric metric = metricSet.get(attribute);
+ if (metric == null) {
+ throw new AttributeNotFoundException("Attribute not found
[attribute=" + attribute + ']');
+ }
if (metric instanceof DoubleMetric) {
return ((DoubleMetric) metric).value();
@@ -81,7 +84,8 @@ public class MetricSetMbean implements DynamicMBean {
return ((UuidGauge) metric).value();
}
- throw new AttributeNotFoundException("Unknown metric class [class=" +
metric.getClass().getName() + ']');
+ throw new AttributeNotFoundException(
+ "Unknown metric class [attribute=" + attribute + ", class=" +
metric.getClass().getName() + ']');
}
@Override
@@ -91,13 +95,10 @@ public class MetricSetMbean implements DynamicMBean {
try {
for (String attribute : attributes) {
+ // getAttribute must return a value of the attribute, not
Attribute instance.
Object val = getAttribute(attribute);
- if (val instanceof Attribute) {
- attrList.add((Attribute) val);
- } else {
- attrList.add(new Attribute(attribute, val));
- }
+ attrList.add(new Attribute(attribute, val));
}
} catch (AttributeNotFoundException e) {
throw new IllegalArgumentException(e);
diff --git
a/modules/metrics/src/test/java/org/apache/ignite/internal/metrics/MetricManagerTest.java
b/modules/metrics/src/test/java/org/apache/ignite/internal/metrics/MetricManagerTest.java
new file mode 100644
index 00000000000..49281fb3691
--- /dev/null
+++
b/modules/metrics/src/test/java/org/apache/ignite/internal/metrics/MetricManagerTest.java
@@ -0,0 +1,113 @@
+/*
+ * 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.internal.metrics;
+
+import static
org.apache.ignite.internal.metrics.exporters.jmx.JmxExporter.JMX_EXPORTER_NAME;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willSucceedFast;
+import static org.apache.ignite.internal.util.IgniteUtils.makeMbeanName;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+
+import java.lang.management.ManagementFactory;
+import java.util.UUID;
+import javax.management.ObjectName;
+import
org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import
org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.manager.ComponentContext;
+import org.apache.ignite.internal.metrics.configuration.MetricConfiguration;
+import org.apache.ignite.internal.metrics.sources.OsMetricSource;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests {@link MetricManager}.
+ */
+@ExtendWith(ConfigurationExtension.class)
+public class MetricManagerTest extends BaseIgniteAbstractTest {
+ private static final IgniteLogger LOG =
Loggers.forClass(MetricManagerTest.class);
+
+ private static final String NODE_NAME = "test-node-name";
+
+ private static final UUID NODE_ID = UUID.randomUUID();
+
+ @InjectConfiguration("mock.exporters = {" + JMX_EXPORTER_NAME + " =
{exporterName = " + JMX_EXPORTER_NAME + "}}")
+ private MetricConfiguration jmxMetricConfiguration;
+
+ @Test
+ void metricSourceRegistration() throws Exception {
+ MetricManager metricManager =
createAndStartManager(jmxMetricConfiguration);
+
+ OsMetricSource source = new OsMetricSource();
+ metricManager.registerSource(source);
+
+ MetricSet metricSet = metricManager.enable(source);
+ assertThat(metricSet, notNullValue());
+
+ ObjectName name = makeMbeanName(NODE_NAME, metricSet.group(),
metricSet.name());
+
assertThat(ManagementFactory.getPlatformMBeanServer().isRegistered(name),
is(true));
+
+ stopManager(metricManager);
+
+
assertThat(ManagementFactory.getPlatformMBeanServer().isRegistered(name),
is(false));
+ }
+
+ @Test
+ void metricSourceCleanup() throws Exception {
+ MetricManager metricManager =
createAndStartManager(jmxMetricConfiguration);
+
+ OsMetricSource source = new OsMetricSource();
+ metricManager.registerSource(source);
+
+ MetricSet metricSet = metricManager.enable(source);
+ assertThat(metricSet, notNullValue());
+
+ ObjectName name = makeMbeanName(NODE_NAME, metricSet.group(),
metricSet.name());
+ assertThat(
+ "Metric source was not registered.",
+ ManagementFactory.getPlatformMBeanServer().isRegistered(name),
+ is(true));
+
+ metricManager.unregisterSource(source);
+
+ assertThat(
+ "Metric source was not disabled before unregistering.",
+ ManagementFactory.getPlatformMBeanServer().isRegistered(name),
+ is(false));
+
+ stopManager(metricManager);
+ }
+
+ private static MetricManager createAndStartManager(MetricConfiguration
configuration) {
+ MetricManager metricManager = new MetricManagerImpl(LOG, null);
+
+ metricManager.configure(configuration, () -> NODE_ID, NODE_NAME);
+
+ assertThat(metricManager.startAsync(new ComponentContext()),
willSucceedFast());
+
+ return metricManager;
+ }
+
+ private static void stopManager(MetricManager metricManager) {
+ metricManager.beforeNodeStop();
+ assertThat(metricManager.stopAsync(new ComponentContext()),
willSucceedFast());
+ }
+}
diff --git
a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
index 928f430d891..6b2ed891f88 100644
---
a/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
+++
b/modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java
@@ -1834,8 +1834,7 @@ public class TableManager implements
IgniteTablesInternal, IgniteComponent {
// Handle missed table drop event.
int tableId = tableDescriptor.id();
- boolean destroyTableEventFired = nextCatalog != null &&
nextCatalog.table(tableId) == null;
- if (destroyTableEventFired) {
+ if (nextCatalog != null && nextCatalog.table(tableId) == null)
{
destructionEventsQueue.enqueue(new
DestroyTableEvent(nextCatalog.version(), tableId));
}
@@ -1853,12 +1852,6 @@ public class TableManager implements
IgniteTablesInternal, IgniteComponent {
schemaDescriptor
);
- if (destroyTableEventFired) {
- // prepareTableResourcesOnRecovery registers a table
metric source, so we need to unregister it here,
- // just because the table is being dropped and there is no
need to keep the metric source.
- unregisterMetricsSource(tables.get(tableId));
- }
-
startTableFutures.add(startTableFuture);
}
@@ -2090,10 +2083,15 @@ public class TableManager implements
IgniteTablesInternal, IgniteComponent {
}
private TableMetricSource
createAndRegisterMetricsSource(StorageTableDescriptor tableDescriptor,
QualifiedName tableName) {
+ // The table might be created during the recovery phase.
+ // In that case, we should only register the metric source for the
actual tables that exist in the latest catalog.
+ boolean registrationNeeded =
+ catalogService.latestCatalog().table(tableDescriptor.getId())
!= null;
+
StorageEngine engine =
dataStorageMgr.engineByStorageProfile(tableDescriptor.getStorageProfile());
// Engine can be null sometimes, see "TableManager.createTableStorage".
- if (engine != null) {
+ if (engine != null && registrationNeeded) {
StorageEngineTablesMetricSource engineMetricSource = new
StorageEngineTablesMetricSource(engine.name(), tableName);
engine.addTableMetrics(tableDescriptor, engineMetricSource);
@@ -2109,11 +2107,13 @@ public class TableManager implements
IgniteTablesInternal, IgniteComponent {
TableMetricSource source = new TableMetricSource(tableName);
- try {
- metricManager.registerSource(source);
- metricManager.enable(source);
- } catch (Exception e) {
- LOG.warn("Failed to register metrics source for table [id={},
name={}].", e, tableDescriptor.getId(), tableName);
+ if (registrationNeeded) {
+ try {
+ metricManager.registerSource(source);
+ metricManager.enable(source);
+ } catch (Exception e) {
+ LOG.warn("Failed to register metrics source for table [id={},
name={}].", e, tableDescriptor.getId(), tableName);
+ }
}
return source;
@@ -2134,6 +2134,7 @@ public class TableManager implements
IgniteTablesInternal, IgniteComponent {
String storageProfile =
table.internalTable().storage().getTableDescriptor().getStorageProfile();
StorageEngine engine =
dataStorageMgr.engineByStorageProfile(storageProfile);
+
// Engine can be null sometimes, see "TableManager.createTableStorage".
if (engine != null) {
try {
