You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Russole (via GitHub)" <gi...@apache.org> on 2026/01/07 13:58:12 UTC
[PR] HDDS-9438. Improve ProtocolMessageMetrics [ozone]
Russole opened a new pull request, #9600:
URL: https://github.com/apache/ozone/pull/9600
## What changes were proposed in this pull request?
* Refactored ProtocolMessageMetrics to use a single unmodifiable EnumMap for per-message metrics.
* Consolidated counters and elapsed time into one per-type structure instead of maintaining multiple maps.
* Reused static metric metadata (MetricsInfo) and removed dynamic creation of metric definitions.
* Updated call sites to align generic type bounds (KEY extends Enum<KEY>) required by the new implementation.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9438
## How was this patch tested?
All CI checks passed.
https://github.com/Russole/ozone/actions/runs/20752706566
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9438. Improve ProtocolMessageMetrics [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #9600:
URL: https://github.com/apache/ozone/pull/9600
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9438. Improve ProtocolMessageMetrics [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9600:
URL: https://github.com/apache/ozone/pull/9600#discussion_r2670142955
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java:
##########
@@ -34,47 +35,56 @@
/**
* Metrics to count all the subtypes of a specific message.
*/
-public class ProtocolMessageMetrics<KEY> implements MetricsSource {
+public class ProtocolMessageMetrics<KEY extends Enum<KEY>> implements MetricsSource {
private final String name;
private final String description;
- private final Map<KEY, AtomicLong> counters =
- new ConcurrentHashMap<>();
-
- private final Map<KEY, AtomicLong> elapsedTimes =
- new ConcurrentHashMap<>();
+ private final Map<KEY, Stats> stats;
private final AtomicInteger concurrency = new AtomicInteger(0);
- public static <KEY> ProtocolMessageMetrics<KEY> create(String name,
+ private static final MetricsInfo TYPE_TAG_INFO =
+ Interns.info("type", "Message type");
+
+ private static final MetricsInfo COUNTER_INFO =
+ Interns.info("counter", "Number of distinct calls");
+
+ private static final MetricsInfo TIME_INFO =
+ Interns.info("time", "Sum of the duration of the calls");
+
+ private static final MetricsInfo CONCURRENCY_INFO =
+ Interns.info("concurrency",
+ "Number of requests processed concurrently");
Review Comment:
The line width is 120 characters. Let's avoid the short lines.
```java
private static final MetricsInfo TYPE_TAG_INFO = Interns.info("type", "Message type");
private static final MetricsInfo COUNTER_INFO = Interns.info("counter", "Number of distinct calls");
private static final MetricsInfo TIME_INFO = Interns.info("time", "Sum of the duration of the calls");
private static final MetricsInfo CONCURRENCY_INFO = Interns.info("concurrency",
"Number of requests processed concurrently");
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java:
##########
@@ -89,44 +99,41 @@ public void unregister() {
@Override
public void getMetrics(MetricsCollector collector, boolean all) {
- counters.forEach((key, value) -> {
+ stats.forEach((key, stat) -> {
MetricsRecordBuilder builder =
collector.addRecord(name);
builder.add(
- new MetricsTag(Interns.info("type", "Message type"), key.toString()));
- builder.addCounter(new MetricName("counter", "Number of distinct calls"),
- value.longValue());
+ new MetricsTag(TYPE_TAG_INFO, key.toString()));
+ builder.addCounter(COUNTER_INFO,
+ stat.counter());
builder.addCounter(
- new MetricName("time", "Sum of the duration of the calls"),
- elapsedTimes.get(key).longValue());
+ TIME_INFO,
+ stat.time());
builder.endRecord();
Review Comment:
Let's combine the short lines:
```java
collector.addRecord(name)
.add(new MetricsTag(TYPE_TAG_INFO, key.toString()))
.addCounter(COUNTER_INFO, stat.counter())
.addCounter(TIME_INFO, stat.time())
.endRecord();
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java:
##########
@@ -34,47 +35,56 @@
/**
* Metrics to count all the subtypes of a specific message.
*/
-public class ProtocolMessageMetrics<KEY> implements MetricsSource {
+public class ProtocolMessageMetrics<KEY extends Enum<KEY>> implements MetricsSource {
Review Comment:
👍 the generic type should extend Enum.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java:
##########
@@ -89,44 +99,41 @@ public void unregister() {
@Override
public void getMetrics(MetricsCollector collector, boolean all) {
- counters.forEach((key, value) -> {
+ stats.forEach((key, stat) -> {
MetricsRecordBuilder builder =
collector.addRecord(name);
builder.add(
- new MetricsTag(Interns.info("type", "Message type"), key.toString()));
- builder.addCounter(new MetricName("counter", "Number of distinct calls"),
- value.longValue());
+ new MetricsTag(TYPE_TAG_INFO, key.toString()));
+ builder.addCounter(COUNTER_INFO,
+ stat.counter());
builder.addCounter(
- new MetricName("time", "Sum of the duration of the calls"),
- elapsedTimes.get(key).longValue());
+ TIME_INFO,
+ stat.time());
builder.endRecord();
});
MetricsRecordBuilder builder = collector.addRecord(name);
- builder.addCounter(new MetricName("concurrency",
- "Number of requests processed concurrently"), concurrency.get());
+ builder.addCounter(CONCURRENCY_INFO, concurrency.get());
Review Comment:
Similarly,
```java
collector.addRecord(name)
.addCounter(CONCURRENCY_INFO, concurrency.get());
```
BTW, do we need `.endRecord();`?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ProtocolMessageMetrics.java:
##########
@@ -34,47 +35,56 @@
/**
* Metrics to count all the subtypes of a specific message.
*/
-public class ProtocolMessageMetrics<KEY> implements MetricsSource {
+public class ProtocolMessageMetrics<KEY extends Enum<KEY>> implements MetricsSource {
private final String name;
private final String description;
- private final Map<KEY, AtomicLong> counters =
- new ConcurrentHashMap<>();
-
- private final Map<KEY, AtomicLong> elapsedTimes =
- new ConcurrentHashMap<>();
+ private final Map<KEY, Stats> stats;
private final AtomicInteger concurrency = new AtomicInteger(0);
- public static <KEY> ProtocolMessageMetrics<KEY> create(String name,
+ private static final MetricsInfo TYPE_TAG_INFO =
+ Interns.info("type", "Message type");
+
+ private static final MetricsInfo COUNTER_INFO =
+ Interns.info("counter", "Number of distinct calls");
+
+ private static final MetricsInfo TIME_INFO =
+ Interns.info("time", "Sum of the duration of the calls");
+
+ private static final MetricsInfo CONCURRENCY_INFO =
+ Interns.info("concurrency",
+ "Number of requests processed concurrently");
+
+ public static <KEY extends Enum<KEY>> ProtocolMessageMetrics<KEY> create(String name,
String description, KEY[] types) {
- return new ProtocolMessageMetrics<KEY>(name, description, types);
+ return new ProtocolMessageMetrics<>(name, description, types);
}
public ProtocolMessageMetrics(String name, String description,
KEY[] values) {
this.name = name;
this.description = description;
+ final Class<KEY> enumClass = values[0].getDeclaringClass();
+ final EnumMap<KEY, Stats> map = new EnumMap<>(enumClass);
for (KEY value : values) {
- counters.put(value, new AtomicLong(0));
- elapsedTimes.put(value, new AtomicLong(0));
+ map.put(value, new Stats());
}
+ this.stats = Collections.unmodifiableMap(map);
}
Review Comment:
- Pass the class instead of values.
- Change the constructor to private.
```java
public static <KEY extends Enum<KEY>> ProtocolMessageMetrics<KEY> create(String name,
String description, Class<KEY> enumClass) {
return new ProtocolMessageMetrics<>(name, description, enumClass);
}
private ProtocolMessageMetrics(String name, String description, Class<KEY> enumClass) {
this.name = name;
this.description = description;
final EnumMap<KEY, Stats> map = new EnumMap<>(enumClass);
for (KEY value : enumClass.getEnumConstants()) {
map.put(value, new Stats());
}
this.stats = Collections.unmodifiableMap(map);
}
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9438. Improve ProtocolMessageMetrics [ozone]
Posted by "Russole (via GitHub)" <gi...@apache.org>.
Russole commented on PR #9600:
URL: https://github.com/apache/ozone/pull/9600#issuecomment-3723998979
Thanks @szetszwo for the reviews.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org