You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "SzyWilliam (via GitHub)" <gi...@apache.org> on 2023/02/20 06:50:52 UTC
[GitHub] [iotdb] SzyWilliam opened a new pull request, #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
SzyWilliam opened a new pull request, #9099:
URL: https://github.com/apache/iotdb/pull/9099
See https://apache-iotdb.feishu.cn/docx/NHsMdWyaJoxjcFxICoCcvDannNb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1112955224
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
Review Comment:
To make each of them independent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115344205
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+
+ return true;
+ }
+
+ void removeAll() {
+ counterCache.forEach((name, counter) -> metricService.remove(MetricType.COUNTER, name));
+ gaugeCache.forEach((name, gauge) -> metricService.remove(MetricType.AUTO_GAUGE, name));
+ timerCache.forEach((name, timer) -> metricService.remove(MetricType.TIMER, name));
+ metricNameCache.clear();
+ counterCache.clear();
+ gaugeCache.clear();
+ timerCache.clear();
+ }
+
+ @Override
+ public Gauge gauge(String name, MetricRegistry.MetricSupplier<Gauge> metricSupplier) {
+ final String fullName = getMetricName(name);
+ final GaugeProxy gauge = new GaugeProxy(metricSupplier);
+ final GaugeProxy previous = gaugeCache.putIfAbsent(fullName, gauge);
+ if (previous == null) {
+ metricService.createAutoGauge(
+ fullName, MetricLevel.IMPORTANT, gauge, GaugeProxy::getValueAsDouble);
+ }
+ return gauge;
+ }
+
+ @Override
+ public Timer timer(String name, MetricRegistry.MetricSupplier<Timer> metricSupplier) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public SortedMap<String, Gauge> getGauges(MetricFilter metricFilter) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public Counter counter(String name, MetricRegistry.MetricSupplier<Counter> metricSupplier) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public Histogram histogram(String name) {
+ throw new UnsupportedOperationException("Histogram is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Meter meter(String name) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Meter meter(String name, MetricRegistry.MetricSupplier<Meter> metricSupplier) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Metric get(String name) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public <T extends Metric> T register(String name, T t) throws IllegalArgumentException {
+ throw new UnsupportedOperationException("register is not used in Ratis Metrics");
+ }
+
+ @Override
+ public MetricRegistry getDropWizardMetricRegistry() {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public MetricRegistryInfo getMetricRegistryInfo() {
+ return info;
+ }
+
+ @Override
+ public void registerAll(String s, MetricSet metricSet) {
+ throw new UnsupportedOperationException("registerAll is not used in Ratis Metrics");
+ }
+
+ @Override
+ public void setJmxReporter(JmxReporter jmxReporter) {
+ // Not Implemented: JmxReporter is not used in Ratis Metrics
+ throw new UnsupportedOperationException("JmxReporter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public JmxReporter getJmxReporter() {
+ throw new UnsupportedOperationException("JmxReporter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public void setConsoleReporter(ConsoleReporter consoleReporter) {
+ // Not Implemented: ConsoleReporter is not used in Ratis Metrics
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1116437746
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/TimerProxy.java:
##########
@@ -30,6 +30,7 @@ public class TimerProxy extends com.codahale.metrics.Timer {
}
@Override
+ // time() method is used as a user time clock. Will reuse the dropwizard implementation.
Review Comment:
Pleause /** / rather than //
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] OneSizeFitsQuorum merged pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum merged PR #9099:
URL: https://github.com/apache/iotdb/pull/9099
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115339978
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/RatisMetricSet.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.metricsets.IMetricSet;
+
+import org.apache.ratis.metrics.MetricRegistries;
+
+public class RatisMetricSet implements IMetricSet {
Review Comment:
This class will be used in subsequent PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#issuecomment-1438354635
Thanks for the reviews @SpriCoder . Made changes on code, please take a look again. Also cc @OneSizeFitsQuorum @BUAAserein
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115338388
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/CounterProxy.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.type.Counter;
+
+/** A Proxy class using IoTDB Counter to replace the dropwizard Counter. */
+public class CounterProxy extends com.codahale.metrics.Counter {
+
+ // IoTDB Counter
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SpriCoder commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SpriCoder (via GitHub)" <gi...@apache.org>.
SpriCoder commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1111633821
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/MetricRegistryManager.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.DoNothingMetricService;
+
+import org.apache.ratis.metrics.MetricRegistries;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+import org.apache.ratis.util.TimeDuration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.function.Consumer;
+
+public class MetricRegistryManager extends MetricRegistries {
+
+ private static final Logger LOG = LoggerFactory.getLogger(MetricRegistryManager.class);
Review Comment:
Maybe we use LOGGER in our project?
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
Review Comment:
If we ignore exceptions here, why use three try?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115338839
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/TimerProxy.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.type.Timer;
+
+import java.util.concurrent.TimeUnit;
+
+public class TimerProxy extends com.codahale.metrics.Timer {
+ private final Timer timer;
+
+ TimerProxy(Timer timer) {
+ this.timer = timer;
+ }
+
+ @Override
+ public Context time() {
+ return super.time();
Review Comment:
The `time` method is only used as a user time clock. Will add comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115346491
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+
+ return true;
+ }
+
+ void removeAll() {
+ counterCache.forEach((name, counter) -> metricService.remove(MetricType.COUNTER, name));
+ gaugeCache.forEach((name, gauge) -> metricService.remove(MetricType.AUTO_GAUGE, name));
+ timerCache.forEach((name, timer) -> metricService.remove(MetricType.TIMER, name));
+ metricNameCache.clear();
+ counterCache.clear();
+ gaugeCache.clear();
+ timerCache.clear();
+ }
+
+ @Override
+ public Gauge gauge(String name, MetricRegistry.MetricSupplier<Gauge> metricSupplier) {
+ final String fullName = getMetricName(name);
+ final GaugeProxy gauge = new GaugeProxy(metricSupplier);
+ final GaugeProxy previous = gaugeCache.putIfAbsent(fullName, gauge);
+ if (previous == null) {
Review Comment:
Thanks for pointing out this! Will switch to computeIfAbsent
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#issuecomment-1441359214
@OneSizeFitsQuorum Thanks a lot for the careful reviews. Made changes on code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] OneSizeFitsQuorum commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1114535914
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/CounterProxy.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.type.Counter;
+
+/** A Proxy class using IoTDB Counter to replace the dropwizard Counter. */
+public class CounterProxy extends com.codahale.metrics.Counter {
+
+ // IoTDB Counter
Review Comment:
use /** */
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/TimerProxy.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.type.Timer;
+
+import java.util.concurrent.TimeUnit;
+
+public class TimerProxy extends com.codahale.metrics.Timer {
+ private final Timer timer;
+
+ TimerProxy(Timer timer) {
+ this.timer = timer;
+ }
+
+ @Override
+ public Context time() {
+ return super.time();
Review Comment:
Why return `super.time()`? In the `update` function we don't seem to update the parent field.
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/RatisMetricSet.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.metricsets.IMetricSet;
+
+import org.apache.ratis.metrics.MetricRegistries;
+
+public class RatisMetricSet implements IMetricSet {
Review Comment:
It looks like this class is not being used?
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/MetricRegistryManager.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.DoNothingMetricService;
+
+import org.apache.ratis.metrics.MetricRegistries;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+import org.apache.ratis.util.TimeDuration;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Consumer;
+
+public class MetricRegistryManager extends MetricRegistries {
+ private final RefCountingMap<MetricRegistryInfo, RatisMetricRegistry> registries;
Review Comment:
Add some comments for why we need to use this `RefCountingMap`
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+
+ return true;
+ }
+
+ void removeAll() {
+ counterCache.forEach((name, counter) -> metricService.remove(MetricType.COUNTER, name));
+ gaugeCache.forEach((name, gauge) -> metricService.remove(MetricType.AUTO_GAUGE, name));
+ timerCache.forEach((name, timer) -> metricService.remove(MetricType.TIMER, name));
+ metricNameCache.clear();
+ counterCache.clear();
+ gaugeCache.clear();
+ timerCache.clear();
+ }
+
+ @Override
+ public Gauge gauge(String name, MetricRegistry.MetricSupplier<Gauge> metricSupplier) {
+ final String fullName = getMetricName(name);
+ final GaugeProxy gauge = new GaugeProxy(metricSupplier);
+ final GaugeProxy previous = gaugeCache.putIfAbsent(fullName, gauge);
+ if (previous == null) {
+ metricService.createAutoGauge(
+ fullName, MetricLevel.IMPORTANT, gauge, GaugeProxy::getValueAsDouble);
+ }
+ return gauge;
+ }
+
+ @Override
+ public Timer timer(String name, MetricRegistry.MetricSupplier<Timer> metricSupplier) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public SortedMap<String, Gauge> getGauges(MetricFilter metricFilter) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public Counter counter(String name, MetricRegistry.MetricSupplier<Counter> metricSupplier) {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public Histogram histogram(String name) {
+ throw new UnsupportedOperationException("Histogram is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Meter meter(String name) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Meter meter(String name, MetricRegistry.MetricSupplier<Meter> metricSupplier) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public Metric get(String name) {
+ throw new UnsupportedOperationException("Meter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public <T extends Metric> T register(String name, T t) throws IllegalArgumentException {
+ throw new UnsupportedOperationException("register is not used in Ratis Metrics");
+ }
+
+ @Override
+ public MetricRegistry getDropWizardMetricRegistry() {
+ throw new UnsupportedOperationException("This method is not used in IoTDB project");
+ }
+
+ @Override
+ public MetricRegistryInfo getMetricRegistryInfo() {
+ return info;
+ }
+
+ @Override
+ public void registerAll(String s, MetricSet metricSet) {
+ throw new UnsupportedOperationException("registerAll is not used in Ratis Metrics");
+ }
+
+ @Override
+ public void setJmxReporter(JmxReporter jmxReporter) {
+ // Not Implemented: JmxReporter is not used in Ratis Metrics
+ throw new UnsupportedOperationException("JmxReporter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public JmxReporter getJmxReporter() {
+ throw new UnsupportedOperationException("JmxReporter is not used in Ratis Metrics");
+ }
+
+ @Override
+ public void setConsoleReporter(ConsoleReporter consoleReporter) {
+ // Not Implemented: ConsoleReporter is not used in Ratis Metrics
Review Comment:
remove this duplicated line
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
Review Comment:
It doesn't make sense to make each component independent because you did nothing after you caught the exception. If you don't even have a debug log, it's really better to have just one try.
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+
+ return true;
+ }
+
+ void removeAll() {
+ counterCache.forEach((name, counter) -> metricService.remove(MetricType.COUNTER, name));
+ gaugeCache.forEach((name, gauge) -> metricService.remove(MetricType.AUTO_GAUGE, name));
+ timerCache.forEach((name, timer) -> metricService.remove(MetricType.TIMER, name));
+ metricNameCache.clear();
+ counterCache.clear();
+ gaugeCache.clear();
+ timerCache.clear();
+ }
+
+ @Override
+ public Gauge gauge(String name, MetricRegistry.MetricSupplier<Gauge> metricSupplier) {
+ final String fullName = getMetricName(name);
+ final GaugeProxy gauge = new GaugeProxy(metricSupplier);
+ final GaugeProxy previous = gaugeCache.putIfAbsent(fullName, gauge);
+ if (previous == null) {
Review Comment:
Please ensure atomicity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115343286
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/IoTDBMetricRegistry.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.utils.MetricLevel;
+import org.apache.iotdb.metrics.utils.MetricType;
+
+import com.codahale.metrics.ConsoleReporter;
+import com.codahale.metrics.Counter;
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Histogram;
+import com.codahale.metrics.JmxReporter;
+import com.codahale.metrics.Meter;
+import com.codahale.metrics.Metric;
+import com.codahale.metrics.MetricFilter;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.MetricSet;
+import com.codahale.metrics.Timer;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+
+import java.util.Map;
+import java.util.SortedMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class IoTDBMetricRegistry implements RatisMetricRegistry {
+
+ private final AbstractMetricService metricService;
+ private final MetricRegistryInfo info;
+ private final String prefix;
+ private final Map<String, String> metricNameCache = new ConcurrentHashMap<>();
+ private final Map<String, CounterProxy> counterCache = new ConcurrentHashMap<>();
+ private final Map<String, TimerProxy> timerCache = new ConcurrentHashMap<>();
+ private final Map<String, GaugeProxy> gaugeCache = new ConcurrentHashMap<>();
+
+ IoTDBMetricRegistry(MetricRegistryInfo info, AbstractMetricService service) {
+ this.info = info;
+ this.metricService = service;
+ prefix =
+ MetricRegistry.name(
+ info.getApplicationName(), info.getMetricsComponentName(), info.getPrefix());
+ }
+
+ private String getMetricName(String name) {
+ return metricNameCache.computeIfAbsent(name, n -> MetricRegistry.name(prefix, n));
+ }
+
+ @Override
+ public Timer timer(String name) {
+ final String fullName = getMetricName(name);
+ return timerCache.computeIfAbsent(
+ fullName, fn -> new TimerProxy(metricService.getOrCreateTimer(fn, MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public Counter counter(String name) {
+ final String fullName = getMetricName(name);
+ return counterCache.computeIfAbsent(
+ fullName,
+ fn ->
+ new CounterProxy(
+ metricService.getOrCreateCounter(getMetricName(name), MetricLevel.IMPORTANT)));
+ }
+
+ @Override
+ public boolean remove(String name) {
+ // TODO (szywilliam) we can add an interface like removeTypeless(name)
+
+ try {
+ metricService.remove(MetricType.COUNTER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.TIMER, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
+ try {
+ metricService.remove(MetricType.AUTO_GAUGE, getMetricName(name));
+ } catch (IllegalArgumentException ignored) {
+ }
Review Comment:
Will add comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #9099: [IOTDB-5559] Implement metric exporters for RatisConsensus
Posted by "SzyWilliam (via GitHub)" <gi...@apache.org>.
SzyWilliam commented on code in PR #9099:
URL: https://github.com/apache/iotdb/pull/9099#discussion_r1115340932
##########
consensus/src/main/java/org/apache/iotdb/consensus/ratis/metrics/MetricRegistryManager.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.iotdb.consensus.ratis.metrics;
+
+import org.apache.iotdb.metrics.AbstractMetricService;
+import org.apache.iotdb.metrics.DoNothingMetricService;
+
+import org.apache.ratis.metrics.MetricRegistries;
+import org.apache.ratis.metrics.MetricRegistryInfo;
+import org.apache.ratis.metrics.RatisMetricRegistry;
+import org.apache.ratis.util.TimeDuration;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Consumer;
+
+public class MetricRegistryManager extends MetricRegistries {
+ private final RefCountingMap<MetricRegistryInfo, RatisMetricRegistry> registries;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org