[GitHub] [storm] Ethanlm commented on a change in pull request #3411: STORM-3793 add metric to track backpressure status for tasks

[GitBox <gi...@apache.org>]

Ethanlm commented on a change in pull request #3411:
URL: https://github.com/apache/storm/pull/3411#discussion_r714037922



##########
File path: storm-client/src/jvm/org/apache/storm/daemon/worker/BackPressureTracker.java
##########
@@ -118,8 +118,26 @@ public void setLastOverflowCount(BackpressureState state, int value) {
         private int lastOverflowCount = 0;
 
 
-        BackpressureState(JCQueue queue) {
+        BackpressureState(JCQueue queue, Integer taskId, String componentId, StormMetricRegistry metricRegistry) {
             this.queue = queue;
+
+            // System bolt is not a part of backpressure.
+            if (taskId >= 0) {
+                if (componentId == null) {
+                    throw new RuntimeException("Missing componentId for task " + taskId);
+                }
+
+                Gauge<Integer> bpOverflowCount = new Gauge<Integer>() {
+                    @Override
+                    public Integer getValue() {
+                        if (backpressure.get()) {
+                            return Math.max(1, lastOverflowCount);
+                        }
+                        return 0;
+                    }
+                };
+                metricRegistry.gauge("__backpressure-overflow-count", bpOverflowCount, componentId, taskId);

Review comment:
       Can we change this to `__backpressure-last-overflow-count`? I think it reflects better the meaning of the metric. And we have a queue `-overflow` metric. Thinking the name change can make it a little bit more clear.




-- 
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