You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/01/28 21:53:44 UTC

[GitHub] [incubator-pinot] fx19880617 opened a new pull request #6502: Adding cron scheduler metrics reporting

fx19880617 opened a new pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502


   ## Description
   Adding metrics for cron scheduler execution:
   ![image](https://user-images.githubusercontent.com/1202120/106202956-8674d580-616f-11eb-8624-1c248d41c59d.png)
   
   
   JMX report generates sample records below:
   ```
   # HELP pinot_controller_cronSchedulerJobExecutionTimeMs_Count Attribute exposed for management ("org.apache.pinot.common.metrics"<type="ControllerMetrics", name="pinot.controller.githubEvents_OFFLINE.SegmentGenerationAndPushTask.cronSchedulerJobExecutionTimeMs"><>Count)
   # TYPE pinot_controller_cronSchedulerJobExecutionTimeMs_Count untyped
   pinot_controller_cronSchedulerJobExecutionTimeMs_Count{table="githubEvents_OFFLINE",taskType="SegmentGenerationAndPushTask",} 1.0
   ```
   


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

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

[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566609465



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -158,6 +150,8 @@ private synchronized void removeAllTasksFromCronExpressions(String tableWithType
       if (jobKey.getName().equals(tableWithType)) {
         try {
           _scheduledExecutorService.deleteJob(jobKey);
+          _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, jobKey.getGroup()),
+              ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -1L);

Review comment:
       make sense, moved it.




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

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

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566470241



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -220,6 +218,8 @@ private void updateCronTaskScheduler(String tableWithType, Map<String, String> t
         if (!taskToCronExpressionMap.containsKey(existingTaskType)) {
           try {
             _scheduledExecutorService.deleteJob(JobKey.jobKey(tableWithType, existingTaskType));
+            _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, existingTaskType),
+                ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -11L);

Review comment:
       What's the meaning of `-11L` here?

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java
##########
@@ -42,11 +45,17 @@ public void execute(JobExecutionContext jobExecutionContext)
             .get(PinotTaskManager.LEAD_CONTROLLER_MANAGER_KEY);
     String table = jobExecutionContext.getJobDetail().getKey().getName();
     String taskType = jobExecutionContext.getJobDetail().getKey().getGroup();
+    pinotTaskManager.getControllerMetrics().addMeteredTableValue(PinotTaskManager.getCronJobName(table, taskType),
+        ControllerMeter.CRON_SCHEDULER_JOB_TRIGGERED, 1);

Review comment:
       nit: use `1L` here

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -290,6 +290,9 @@ private void scheduleJob(String tableWithType, String taskType, String cronExprS
               .build();
       try {
         _scheduledExecutorService.scheduleJob(jobDetail, trigger);
+        _controllerMetrics

Review comment:
       Might be good to log a message here?




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

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

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#issuecomment-769453155


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=h1) Report
   > Merging [#6502](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=desc) (4b9d9a8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.45%`.
   > The diff coverage is `41.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6502/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #6502       +/-   ##
   ===========================================
   - Coverage   66.44%   43.98%   -22.46%     
   ===========================================
     Files        1075     1334      +259     
     Lines       54773    65798    +11025     
     Branches     8168     9605     +1437     
   ===========================================
   - Hits        36396    28944     -7452     
   - Misses      15700    34416    +18716     
   + Partials     2677     2438      -239     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `43.98% <41.96%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...r/routing/segmentpruner/interval/IntervalTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsVHJlZS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [1337 more](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=footer). Last update [cf35e6e...4b9d9a8](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566565244



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -220,6 +218,8 @@ private void updateCronTaskScheduler(String tableWithType, Map<String, String> t
         if (!taskToCronExpressionMap.containsKey(existingTaskType)) {
           try {
             _scheduledExecutorService.deleteJob(JobKey.jobKey(tableWithType, existingTaskType));
+            _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, existingTaskType),
+                ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -11L);

Review comment:
       I mean why the number `-11L` is used here. Should it be `-1L`?




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

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

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566598816



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -220,6 +218,8 @@ private void updateCronTaskScheduler(String tableWithType, Map<String, String> t
         if (!taskToCronExpressionMap.containsKey(existingTaskType)) {
           try {
             _scheduledExecutorService.deleteJob(JobKey.jobKey(tableWithType, existingTaskType));
+            _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, existingTaskType),
+                ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -11L);

Review comment:
       If `-11L` is meaningful, could you put a comment on its description?




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

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

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566600230



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -158,6 +150,8 @@ private synchronized void removeAllTasksFromCronExpressions(String tableWithType
       if (jobKey.getName().equals(tableWithType)) {
         try {
           _scheduledExecutorService.deleteJob(jobKey);
+          _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, jobKey.getGroup()),
+              ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -1L);

Review comment:
       I feel that `CRON_SCHEDULER_JOB_SCHEDULED` be more of a gauge other than a meter. E.g., meter is measured on per minute basis. Gauge is like a variable storing the current value.




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

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

[GitHub] [incubator-pinot] codecov-io commented on pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#issuecomment-769453155


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=h1) Report
   > Merging [#6502](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=desc) (4492e90) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.55%`.
   > The diff coverage is `41.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6502/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #6502       +/-   ##
   ===========================================
   - Coverage   66.44%   43.89%   -22.56%     
   ===========================================
     Files        1075     1334      +259     
     Lines       54773    65635    +10862     
     Branches     8168     9571     +1403     
   ===========================================
   - Hits        36396    28808     -7588     
   - Misses      15700    34408    +18708     
   + Partials     2677     2419      -258     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `43.89% <41.96%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...r/routing/segmentpruner/interval/IntervalTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsVHJlZS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [1334 more](https://codecov.io/gh/apache/incubator-pinot/pull/6502/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=footer). Last update [cf35e6e...4492e90](https://codecov.io/gh/apache/incubator-pinot/pull/6502?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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

[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566557500



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -220,6 +218,8 @@ private void updateCronTaskScheduler(String tableWithType, Map<String, String> t
         if (!taskToCronExpressionMap.containsKey(existingTaskType)) {
           try {
             _scheduledExecutorService.deleteJob(JobKey.jobKey(tableWithType, existingTaskType));
+            _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, existingTaskType),
+                ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -11L);

Review comment:
       it's tracking how many scheduled jobs in the controller.




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

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

[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502#discussion_r566558025



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
##########
@@ -290,6 +290,9 @@ private void scheduleJob(String tableWithType, String taskType, String cronExprS
               .build();
       try {
         _scheduledExecutorService.scheduleJob(jobDetail, trigger);
+        _controllerMetrics

Review comment:
       done.

##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java
##########
@@ -42,11 +45,17 @@ public void execute(JobExecutionContext jobExecutionContext)
             .get(PinotTaskManager.LEAD_CONTROLLER_MANAGER_KEY);
     String table = jobExecutionContext.getJobDetail().getKey().getName();
     String taskType = jobExecutionContext.getJobDetail().getKey().getGroup();
+    pinotTaskManager.getControllerMetrics().addMeteredTableValue(PinotTaskManager.getCronJobName(table, taskType),
+        ControllerMeter.CRON_SCHEDULER_JOB_TRIGGERED, 1);

Review comment:
       done




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

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

[GitHub] [incubator-pinot] fx19880617 merged pull request #6502: Adding cron scheduler metrics reporting

Posted by GitBox <gi...@apache.org>.
fx19880617 merged pull request #6502:
URL: https://github.com/apache/incubator-pinot/pull/6502


   


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

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