[core] Support remove metric groups from metric instance (apache#2062)

* [core] Support remove metric groups from metric instance
choucmei · Sep 28, 2023 · 8ee4d1a · 8ee4d1a
1 parent ae1dde1
commit 8ee4d1a
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 8 deletions.
diff --git a/paimon-core/src/main/java/org/apache/paimon/metrics/AbstractMetricGroup.java b/paimon-core/src/main/java/org/apache/paimon/metrics/AbstractMetricGroup.java
@@ -183,6 +183,7 @@ public void close() {
         if (!closed) {
             closed = true;
             metrics.clear();
+            Metrics.getInstance().removeGroup(this);
         }
     }
 

diff --git a/paimon-core/src/main/java/org/apache/paimon/metrics/Metrics.java b/paimon-core/src/main/java/org/apache/paimon/metrics/Metrics.java
@@ -44,6 +44,11 @@ public void addGroup(AbstractMetricGroup group) {
         metricGroups.add(group);
     }
 
+    /** Remove a metric group. Called when closing the corresponding instances, like committer. */
+    public void removeGroup(AbstractMetricGroup group) {
+        metricGroups.remove(group);
+    }
+
     /** Get metric groups. */
     public ConcurrentLinkedQueue<MetricGroup> getMetricGroups() {
         return metricGroups;

diff --git a/paimon-core/src/test/java/org/apache/paimon/metrics/MetricGroupTest.java b/paimon-core/src/test/java/org/apache/paimon/metrics/MetricGroupTest.java
@@ -34,14 +34,7 @@ public void testGroupRegisterMetrics() {
         assertThat(group.isClosed()).isFalse();
         // these will fail is the registration is propagated
         group.counter("testcounter");
-        group.gauge(
-                "testgauge",
-                new Gauge<Object>() {
-                    @Override
-                    public Object getValue() {
-                        return null;
-                    }
-                });
+        group.gauge("testgauge", () -> null);
         assertThat(group.getGroupName()).isEqualTo("commit");
         assertThat(group.getAllTags().size()).isEqualTo(1);
         assertThat(group.getAllTags())
@@ -65,5 +58,15 @@ public void testTolerateMetricNameCollisions() {
 
         // return the old one with the metric name collision
         assertThat(group.counter(name)).isSameAs(counter1);
+        group.close();
+    }
+
+    @Test
+    public void testAddAndRemoveMetricGroups() {
+        AbstractMetricGroup metricGroup =
+                GenericMetricGroup.createGenericMetricGroup("myTable", "commit");
+        assertThat(Metrics.getInstance().getMetricGroups()).containsExactly(metricGroup);
+        metricGroup.close();
+        assertThat(Metrics.getInstance().getMetricGroups()).isEmpty();
     }
 }