You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by xi...@apache.org on 2022/10/28 08:22:00 UTC
[iotdb] branch master updated: [IOTDB-4779] Fix remove metrics in Metric Module (#7762)
This is an automated email from the ASF dual-hosted git repository.
xingtanzjr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/master by this push:
new 6d1dff2421 [IOTDB-4779] Fix remove metrics in Metric Module (#7762)
6d1dff2421 is described below
commit 6d1dff242137b3518d8b2eae9a4d6690e0026a7a
Author: ZhangHongYin <46...@users.noreply.github.com>
AuthorDate: Fri Oct 28 16:21:55 2022 +0800
[IOTDB-4779] Fix remove metrics in Metric Module (#7762)
---
.../dropwizard/DropwizardMetricManager.java | 2 +-
.../iotdb/metrics/AbstractMetricManager.java | 47 +++++++++-------------
.../iotdb/metrics/impl/DoNothingMetricManager.java | 2 +-
.../org/apache/iotdb/metrics/utils/MetricInfo.java | 2 +-
.../micrometer/MicrometerMetricManager.java | 2 +-
.../apache/iotdb/db/metric/MetricServiceTest.java | 19 +++++++++
6 files changed, 43 insertions(+), 31 deletions(-)
diff --git a/metrics/dropwizard-metrics/src/main/java/org/apache/iotdb/metrics/dropwizard/DropwizardMetricManager.java b/metrics/dropwizard-metrics/src/main/java/org/apache/iotdb/metrics/dropwizard/DropwizardMetricManager.java
index 693c10f506..d8ebcfa71d 100644
--- a/metrics/dropwizard-metrics/src/main/java/org/apache/iotdb/metrics/dropwizard/DropwizardMetricManager.java
+++ b/metrics/dropwizard-metrics/src/main/java/org/apache/iotdb/metrics/dropwizard/DropwizardMetricManager.java
@@ -91,7 +91,7 @@ public class DropwizardMetricManager extends AbstractMetricManager {
}
@Override
- protected void remove(MetricType type, MetricInfo metricInfo) {
+ protected void removeMetric(MetricType type, MetricInfo metricInfo) {
metricRegistry.remove(DropwizardMetricNameTool.toFlatString(metricInfo));
}
diff --git a/metrics/interface/src/main/java/org/apache/iotdb/metrics/AbstractMetricManager.java b/metrics/interface/src/main/java/org/apache/iotdb/metrics/AbstractMetricManager.java
index f1773c4106..ec1c764b67 100644
--- a/metrics/interface/src/main/java/org/apache/iotdb/metrics/AbstractMetricManager.java
+++ b/metrics/interface/src/main/java/org/apache/iotdb/metrics/AbstractMetricManager.java
@@ -47,13 +47,13 @@ public abstract class AbstractMetricManager {
/** Is metric service enabled */
protected static boolean isEnableMetric;
/** metric name -> tag keys */
- protected Map<String, MetricInfo.MetaInfo> nameToTagInfo;
+ protected Map<String, MetricInfo.MetaInfo> nameToMetaInfo;
/** metric type -> metric name -> metric info */
protected Map<MetricInfo, IMetric> metrics;
public AbstractMetricManager() {
isEnableMetric = METRIC_CONFIG.getEnableMetric();
- nameToTagInfo = new ConcurrentHashMap<>();
+ nameToMetaInfo = new ConcurrentHashMap<>();
metrics = new ConcurrentHashMap<>();
}
@@ -75,7 +75,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Counter counter = createCounter(metricInfo);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return counter;
});
if (metric instanceof Counter) {
@@ -110,7 +110,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Gauge gauge = createAutoGauge(metricInfo, obj, mapper);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return gauge;
});
if (metric instanceof Gauge) {
@@ -140,7 +140,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Gauge gauge = createGauge(metricInfo);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return gauge;
});
if (metric instanceof Gauge) {
@@ -169,7 +169,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Rate rate = createRate(metricInfo);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return rate;
});
if (metric instanceof Rate) {
@@ -198,7 +198,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Histogram histogram = createHistogram(metricInfo);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return histogram;
});
if (metric instanceof Histogram) {
@@ -227,7 +227,7 @@ public abstract class AbstractMetricManager {
metricInfo,
key -> {
Timer timer = createTimer(metricInfo);
- nameToTagInfo.put(name, metricInfo.getMetaInfo());
+ nameToMetaInfo.put(name, metricInfo.getMetaInfo());
return timer;
});
if (metric instanceof Timer) {
@@ -415,22 +415,20 @@ public abstract class AbstractMetricManager {
public void remove(MetricType type, String name, String... tags) {
if (isEnableMetric()) {
MetricInfo metricInfo = new MetricInfo(type, name, tags);
- if (nameToTagInfo.containsKey(metricInfo.getName())) {
- MetricInfo.MetaInfo metaInfo = nameToTagInfo.get(metricInfo.getName());
- if (metaInfo.hasSameKey(tags)) {
- if (type == metaInfo.getType()) {
- remove(metricInfo);
- remove(type, metricInfo);
- } else {
- throw new IllegalArgumentException(
- metricInfo + " failed to remove because the mismatch of type. ");
- }
+ if (metrics.containsKey(metricInfo)) {
+ if (type == metricInfo.getMetaInfo().getType()) {
+ nameToMetaInfo.remove(metricInfo.getName());
+ metrics.remove(metricInfo);
+ removeMetric(type, metricInfo);
+ } else {
+ throw new IllegalArgumentException(
+ metricInfo + " failed to remove because the mismatch of type. ");
}
}
}
}
- protected abstract void remove(MetricType type, MetricInfo metricInfo);
+ protected abstract void removeMetric(MetricType type, MetricInfo metricInfo);
// endregion
@@ -449,7 +447,7 @@ public abstract class AbstractMetricManager {
protected boolean stop() {
isEnableMetric = METRIC_CONFIG.getEnableMetric();
metrics = new ConcurrentHashMap<>();
- nameToTagInfo = new ConcurrentHashMap<>();
+ nameToMetaInfo = new ConcurrentHashMap<>();
return stopFramework();
}
@@ -459,18 +457,13 @@ public abstract class AbstractMetricManager {
if (!isEnableMetricInGivenLevel(metricLevel)) {
return false;
}
- if (!nameToTagInfo.containsKey(name)) {
+ if (!nameToMetaInfo.containsKey(name)) {
return true;
}
- MetricInfo.MetaInfo metaInfo = nameToTagInfo.get(name);
+ MetricInfo.MetaInfo metaInfo = nameToMetaInfo.get(name);
return metaInfo.hasSameKey(tags);
}
- private void remove(MetricInfo metricInfo) {
- nameToTagInfo.remove(metricInfo.getName());
- metrics.remove(metricInfo);
- }
-
@Override
public boolean equals(Object o) {
if (this == o) {
diff --git a/metrics/interface/src/main/java/org/apache/iotdb/metrics/impl/DoNothingMetricManager.java b/metrics/interface/src/main/java/org/apache/iotdb/metrics/impl/DoNothingMetricManager.java
index dbad7741fb..3ee9f2e2f9 100644
--- a/metrics/interface/src/main/java/org/apache/iotdb/metrics/impl/DoNothingMetricManager.java
+++ b/metrics/interface/src/main/java/org/apache/iotdb/metrics/impl/DoNothingMetricManager.java
@@ -80,7 +80,7 @@ public class DoNothingMetricManager extends AbstractMetricManager {
}
@Override
- protected void remove(MetricType type, MetricInfo metricInfo) {
+ protected void removeMetric(MetricType type, MetricInfo metricInfo) {
// do nothing
}
diff --git a/metrics/interface/src/main/java/org/apache/iotdb/metrics/utils/MetricInfo.java b/metrics/interface/src/main/java/org/apache/iotdb/metrics/utils/MetricInfo.java
index 3439b5c9ff..333604cbc7 100644
--- a/metrics/interface/src/main/java/org/apache/iotdb/metrics/utils/MetricInfo.java
+++ b/metrics/interface/src/main/java/org/apache/iotdb/metrics/utils/MetricInfo.java
@@ -137,7 +137,7 @@ public class MetricInfo {
this.tagNames = tagNames;
}
- /** check whether the key in tags is same */
+ /** check whether the keys of tags are same */
public boolean hasSameKey(String... tags) {
if (tags.length != tagNames.size() * 2) {
return false;
diff --git a/metrics/micrometer-metrics/src/main/java/org/apache/iotdb/metrics/micrometer/MicrometerMetricManager.java b/metrics/micrometer-metrics/src/main/java/org/apache/iotdb/metrics/micrometer/MicrometerMetricManager.java
index cef5630f52..a1c6eddcb0 100644
--- a/metrics/micrometer-metrics/src/main/java/org/apache/iotdb/metrics/micrometer/MicrometerMetricManager.java
+++ b/metrics/micrometer-metrics/src/main/java/org/apache/iotdb/metrics/micrometer/MicrometerMetricManager.java
@@ -96,7 +96,7 @@ public class MicrometerMetricManager extends AbstractMetricManager {
}
@Override
- protected void remove(MetricType type, MetricInfo metricInfo) {
+ protected void removeMetric(MetricType type, MetricInfo metricInfo) {
Meter.Type meterType = transformType(type);
Meter.Id id =
new Meter.Id(
diff --git a/server/src/test/java/org/apache/iotdb/db/metric/MetricServiceTest.java b/server/src/test/java/org/apache/iotdb/db/metric/MetricServiceTest.java
index a7db0093f5..a034e2d0d5 100644
--- a/server/src/test/java/org/apache/iotdb/db/metric/MetricServiceTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/metric/MetricServiceTest.java
@@ -252,11 +252,30 @@ public class MetricServiceTest {
metricService.remove(MetricType.TIMER, "timer6", "tag", "value");
assertEquals(4, metricService.getAllTimers().size());
assertEquals(20, metricService.getAllMetricKeys().size());
+
+ // test remove same key and different value counter
+ Counter removeCounter1 =
+ metricService.getOrCreateCounter("remove", MetricLevel.IMPORTANT, "tag", "value1");
+ assertNotNull(removeCounter1);
+ Counter removeCounter2 =
+ metricService.getOrCreateCounter("remove", MetricLevel.IMPORTANT, "tag", "value2");
+ assertNotNull(removeCounter2);
+ assertEquals(6, metricService.getAllCounters().size());
+ assertEquals(22, metricService.getAllMetricKeys().size());
+ metricService.remove(MetricType.COUNTER, "remove", "tag", "value1");
+ assertEquals(5, metricService.getAllCounters().size());
+ assertEquals(21, metricService.getAllMetricKeys().size());
+ removeCounter2 =
+ metricService.getOrCreateCounter("remove", MetricLevel.IMPORTANT, "tag", "value1");
+ assertNotNull(removeCounter2);
+ assertEquals(6, metricService.getAllCounters().size());
+ assertEquals(22, metricService.getAllMetricKeys().size());
}
private void testOtherSituation() {
assertThrows(IllegalArgumentException.class, this::getOrCreateDifferentMetricsWithSameName);
+ // forbidden to register same name but different type metrics
Timer timer =
metricService.getOrCreateTimer("same_name", MetricLevel.IMPORTANT, "tag", "value");
assertNotNull(timer);