You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2021/07/02 18:28:25 UTC
[GitHub] [storm] agresch opened a new pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
agresch opened a new pull request #3402:
URL: https://github.com/apache/storm/pull/3402
## What is the purpose of the change
Updates metrics to V2 API.
## How was the change tested
Ran storm-client unit tests. Validated metrics log for word count topology.
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch commented on pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
agresch commented on pull request #3402:
URL: https://github.com/apache/storm/pull/3402#issuecomment-1004307630
The recv-iconnection metrics are using a deprecated (v1) metrics API. This JIRA is to update them to the newer v2 API.
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3402:
URL: https://github.com/apache/storm/pull/3402#discussion_r664878945
##########
File path: storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java
##########
@@ -71,24 +74,26 @@ public void recv(List<TaskMessage> batch) {
cb.transfer(ret);
}
- /**
- * Returns serialized byte count traffic metrics.
- *
- * @return Map of metric counts, or null if disabled
- */
@Override
- public Object getValueAndReset() {
+ public void registerMetrics(StormMetricRegistry metricRegistry) {
if (!sizeMetricsEnabled) {
- return null;
+ return;
}
- HashMap<String, Long> outMap = new HashMap<>();
- for (Map.Entry<String, AtomicLong> ent : byteCounts.entrySet()) {
- AtomicLong count = ent.getValue();
- if (count.get() > 0) {
- outMap.put(ent.getKey(), count.getAndSet(0L));
+
+ Gauge<Map<String, Long>> sizes = new Gauge<Map<String, Long>>() {
+ @Override
+ public Map<String, Long> getValue() {
+ HashMap<String, Long> counts = new HashMap<>();
+ for (Map.Entry<String, AtomicLong> ent : byteCounts.entrySet()) {
+ AtomicLong count = ent.getValue();
+ if (count.get() > 0) {
+ counts.put(ent.getKey(), count.getAndSet(0L));
Review comment:
Looks like this won't support multiple reporters?
In the case of multiple reporters being configured, this `getAndSet` will return differently
##########
File path: storm-client/src/jvm/org/apache/storm/messaging/netty/Server.java
##########
@@ -118,6 +119,30 @@
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
+
+ if (metricRegistry != null) {
+ Gauge<Map<String, Integer>> enqueued = new Gauge<Map<String, Integer>>() {
+ @Override
+ public Map<String, Integer> getValue() {
Review comment:
Does it support having multiple reporters?
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch closed pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
agresch closed pull request #3402:
URL: https://github.com/apache/storm/pull/3402
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] qiaodaimadelaowang commented on pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
qiaodaimadelaowang commented on pull request #3402:
URL: https://github.com/apache/storm/pull/3402#issuecomment-999373892
I am a newcomer to Storm, I have a question to ask, how did STORM-3781 come from?
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch commented on a change in pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3402:
URL: https://github.com/apache/storm/pull/3402#discussion_r668220037
##########
File path: storm-client/src/jvm/org/apache/storm/messaging/DeserializingConnectionCallback.java
##########
@@ -71,24 +74,26 @@ public void recv(List<TaskMessage> batch) {
cb.transfer(ret);
}
- /**
- * Returns serialized byte count traffic metrics.
- *
- * @return Map of metric counts, or null if disabled
- */
@Override
- public Object getValueAndReset() {
+ public void registerMetrics(StormMetricRegistry metricRegistry) {
if (!sizeMetricsEnabled) {
- return null;
+ return;
}
- HashMap<String, Long> outMap = new HashMap<>();
- for (Map.Entry<String, AtomicLong> ent : byteCounts.entrySet()) {
- AtomicLong count = ent.getValue();
- if (count.get() > 0) {
- outMap.put(ent.getKey(), count.getAndSet(0L));
+
+ Gauge<Map<String, Long>> sizes = new Gauge<Map<String, Long>>() {
+ @Override
+ public Map<String, Long> getValue() {
+ HashMap<String, Long> counts = new HashMap<>();
+ for (Map.Entry<String, AtomicLong> ent : byteCounts.entrySet()) {
+ AtomicLong count = ent.getValue();
+ if (count.get() > 0) {
+ counts.put(ent.getKey(), count.getAndSet(0L));
Review comment:
I don't see a good way to support that with the V2 interface. Possibly we could provide hooks to allow the metrics to know what reporters exist and when they report. Then they could defer resetting until all reporters have reported. But even then there could be frequency differences between multiple reporters.
We could leave these as V1 metrics if that is preferred. I am not sure how often these are used.
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch commented on pull request #3402: STORM-3781 switch recv-iconnection to v2 metric api
Posted by GitBox <gi...@apache.org>.
agresch commented on pull request #3402:
URL: https://github.com/apache/storm/pull/3402#issuecomment-1081105344
closing for now
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org