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/09/08 19:27:02 UTC
[GitHub] [storm] agresch opened a new pull request #3411: STORM-3793 add metric to track backpressure status for tasks
agresch opened a new pull request #3411:
URL: https://github.com/apache/storm/pull/3411
## What is the purpose of the change
Be able to track which task is causing backpressure.
## How was the change tested
Built code, ran storm-client unit tests, validated metric reports on a dev cluster.
--
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 #3411: STORM-3793 add metric to track backpressure status for tasks
Posted by 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
[GitHub] [storm] agresch commented on a change in pull request #3411: STORM-3793 add metric to track backpressure status for tasks
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3411:
URL: https://github.com/apache/storm/pull/3411#discussion_r714038485
##########
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:
ok
--
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 #3411: STORM-3793 add metric to track backpressure status for tasks
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3411:
URL: https://github.com/apache/storm/pull/3411#discussion_r714057775
##########
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:
Thanks
--
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 #3411: STORM-3793 add metric to track backpressure status for tasks
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3411:
URL: https://github.com/apache/storm/pull/3411#discussion_r714050012
##########
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:
updated
--
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 merged pull request #3411: STORM-3793 add metric to track backpressure status for tasks
Posted by GitBox <gi...@apache.org>.
agresch merged pull request #3411:
URL: https://github.com/apache/storm/pull/3411
--
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