[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers

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

mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660152383



##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -108,6 +120,8 @@ public TaskResult run() {
                 minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FAILED, 1L);
                 LOGGER.error("Caught exception while executing task: {}", _taskConfig.getId(), e);
                 return new TaskResult(TaskResult.Status.FAILED, e.toString());
+              } finally {
+                minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, -1L);

Review comment:
       Best to call a method `runInternal` that has the current logic, and manipulate this metric before and after the call outside (in the run() method). That way, if someone adds a return from an earlier point, we don't have to worry about the metric going bad.




-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org