wang-jiahua commented on code in PR #10443:
URL: https://github.com/apache/rocketmq/pull/10443#discussion_r3389168535
##########
broker/src/main/java/org/apache/rocketmq/broker/metrics/BrokerMetricsManager.java:
##########
@@ -195,12 +202,31 @@ public AttributesBuilder newAttributesBuilder() {
return attributesBuilder;
}
+ public Attributes getOrBuildTopicAttributes(String topic, String
messageType, boolean isSystem) {
+ Attributes lastAttrs = this.lastTopicAttributes;
+ if (lastAttrs != null && topic.equals(this.lastTopicName) &&
messageType.equals(this.lastTopicMsgType)) {
+ return lastAttrs;
+ }
+ String cacheKey = topic + '|' + messageType;
+ Attributes attrs = topicAttributesCache.computeIfAbsent(cacheKey, k ->
+ newAttributesBuilder()
+ .put(LABEL_TOPIC_KEY, topic)
+ .put(LABEL_MESSAGE_TYPE_KEY, messageType)
+ .put(LABEL_IS_SYSTEM_KEY, isSystem)
+ .build()
+ );
+ this.lastTopicName = topic;
+ this.lastTopicMsgType = messageType;
+ this.lastTopicAttributes = attrs;
+ return attrs;
+ }
Review Comment:
Fixed: `isSystem` is now included in both the inline cache comparison
(`isSystem == this.lastTopicIsSystem`) and the `ConcurrentHashMap` cache key
(`topic + "|" + messageType + "|" + isSystem`). Calls with different `isSystem`
values for the same `(topic, messageType)` pair will correctly return different
Attributes.
##########
common/src/main/java/org/apache/rocketmq/common/attribute/TopicMessageType.java:
##########
@@ -33,9 +33,11 @@ public enum TopicMessageType {
MIXED("MIXED");
private final String value;
+ private final String metricsValue;
TopicMessageType(String value) {
this.value = value;
+ this.metricsValue = value.toLowerCase();
}
Review Comment:
Fixed: `metricsValue` is now computed at enum construction time with
`value.toLowerCase(Locale.ROOT)`, making it locale-independent and stable
across environments.
##########
common/src/main/java/org/apache/rocketmq/common/attribute/TopicMessageType.java:
##########
@@ -67,6 +69,6 @@ public static TopicMessageType
parseFromMessageProperty(Map<String, String> mess
}
public String getMetricsValue() {
- return value.toLowerCase();
+ return metricsValue;
}
Review Comment:
Same fix as above — `metricsValue` field uses `toLowerCase(Locale.ROOT)` at
construction time.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
