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

[incubator-pinot] branch adding_cron_scheduler_metrics created (now 4492e90)

This is an automated email from the ASF dual-hosted git repository.

xiangfu pushed a change to branch adding_cron_scheduler_metrics
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


      at 4492e90  Adding cron scheduler metrics reporting

This branch includes the following new commits:

     new 4492e90  Adding cron scheduler metrics reporting

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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


[incubator-pinot] 01/01: Adding cron scheduler metrics reporting

Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

xiangfu pushed a commit to branch adding_cron_scheduler_metrics
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 4492e901838d8c7ada3cd0982f3f6268bda120b6
Author: Xiang Fu <fx...@gmail.com>
AuthorDate: Thu Jan 28 12:35:22 2021 -0800

    Adding cron scheduler metrics reporting
---
 .../etc/jmx_prometheus_javaagent/configs/pinot.yml    | 15 +++++++++++++++
 .../apache/pinot/common/metrics/ControllerMeter.java  |  4 +++-
 .../apache/pinot/common/metrics/ControllerTimer.java  |  2 +-
 .../helix/core/minion/CronJobScheduleJob.java         |  9 +++++++++
 .../helix/core/minion/PinotTaskManager.java           | 19 +++++++++++--------
 .../core/periodictask/ControllerPeriodicTask.java     |  4 ++++
 6 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml
index 312512c..787d340 100644
--- a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml
+++ b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/pinot.yml
@@ -52,6 +52,21 @@ rules:
   name: "pinot_controller_validateion_$2_$3"
   labels:
     table: "$1"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ControllerMetrics\", name=\"pinot.controller.(\\w+)\\.(\\w+).cronSchedulerJobScheduled\"><>(\\w+)"
+  name: "pinot_controller_cronSchedulerJobScheduled_$3"
+  labels:
+    table: "$1"
+    taskType: "$2"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ControllerMetrics\", name=\"pinot.controller.(\\w+)\\.(\\w+).cronSchedulerTriggered\"><>(\\w+)"
+  name: "pinot_controller_cronSchedulerTriggered_$3"
+  labels:
+    table: "$1"
+    taskType: "$2"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ControllerMetrics\", name=\"pinot.controller.(\\w+)\\.(\\w+).cronSchedulerJobExecutionTimeMs\"><>(\\w+)"
+  name: "pinot_controller_cronSchedulerJobExecutionTimeMs_$3"
+  labels:
+    table: "$1"
+    taskType: "$2"
 # Pinot Broker
 - pattern: "\"org.apache.pinot.common.metrics\"<type=\"BrokerMetrics\", name=\"pinot.broker.(\\w+).authorization\"><>(\\w+)"
   name: "pinot_broker_authorization_$2"
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
index 1e7fc46..0caf005 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java
@@ -48,7 +48,9 @@ public enum ControllerMeter implements AbstractMetrics.Meter {
   CONTROLLER_PERIODIC_TASK_ERROR("periodicTaskError", false),
   NUMBER_TIMES_SCHEDULE_TASKS_CALLED("tasks", true),
   NUMBER_TASKS_SUBMITTED("tasks", false),
-  NUMBER_SEGMENT_UPLOAD_TIMEOUT_EXCEEDED("SegmentUploadTimeouts", true);
+  NUMBER_SEGMENT_UPLOAD_TIMEOUT_EXCEEDED("SegmentUploadTimeouts", true),
+  CRON_SCHEDULER_JOB_SCHEDULED("cronSchedulerJobScheduled", false),
+  CRON_SCHEDULER_JOB_TRIGGERED("cronSchedulerJobTriggered", false);
 
   private final String brokerMeterName;
   private final String unit;
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java
index 7617f5a..40ad9d5 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java
@@ -26,7 +26,7 @@ import org.apache.pinot.common.Utils;
  *
  */
 public enum ControllerTimer implements AbstractMetrics.Timer {
-  ;
+  CRON_SCHEDULER_JOB_EXECUTION_TIME_MS("cronSchedulerJobExecutionTimeMs", false);
 
   private final String timerName;
   private final boolean global;
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java
index e5653f4..553d326 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/CronJobScheduleJob.java
@@ -18,6 +18,9 @@
  */
 package org.apache.pinot.controller.helix.core.minion;
 
+import java.util.concurrent.TimeUnit;
+import org.apache.pinot.common.metrics.ControllerMeter;
+import org.apache.pinot.common.metrics.ControllerTimer;
 import org.apache.pinot.controller.LeadControllerManager;
 import org.quartz.Job;
 import org.quartz.JobExecutionContext;
@@ -42,11 +45,17 @@ public class CronJobScheduleJob implements Job {
             .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);
     if (leadControllerManager.isLeaderForTable(table)) {
+      long jobStartTime = System.currentTimeMillis();
       LOGGER.info("Execute CronJob: table - {}, task - {} at {}", table, taskType, jobExecutionContext.getFireTime());
       pinotTaskManager.scheduleTask(taskType, table);
       LOGGER.info("Finished CronJob: table - {}, task - {}, next runtime is {}", table, taskType,
           jobExecutionContext.getNextFireTime());
+      pinotTaskManager.getControllerMetrics().addTimedTableValue(PinotTaskManager.getCronJobName(table, taskType),
+          ControllerTimer.CRON_SCHEDULER_JOB_EXECUTION_TIME_MS, (System.currentTimeMillis() - jobStartTime),
+          TimeUnit.MILLISECONDS);
     } else {
       LOGGER.info("Not Lead, skip processing CronJob: table - {}, task - {}", table, taskType);
     }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
index 436f10f..b5298e9 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java
@@ -124,14 +124,6 @@ public class PinotTaskManager extends ControllerPeriodicTask<Void> {
     return TABLE_CONFIG_PATH_PREFIX + tableWithType;
   }
 
-  protected synchronized void cleanupCronTaskScheduler() {
-    try {
-      _scheduledExecutorService.clear();
-    } catch (SchedulerException e) {
-      LOGGER.error("Failed to clear all tasks in scheduler", e);
-    }
-  }
-
   public synchronized void cleanUpCronTaskSchedulerForTable(String tableWithType) {
     LOGGER.info("Cleaning up task in scheduler for table {}", tableWithType);
     TableTaskSchedulerUpdater tableTaskSchedulerUpdater = _tableTaskSchedulerUpdaterMap.get(tableWithType);
@@ -158,6 +150,8 @@ public class PinotTaskManager extends ControllerPeriodicTask<Void> {
       if (jobKey.getName().equals(tableWithType)) {
         try {
           _scheduledExecutorService.deleteJob(jobKey);
+          _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, jobKey.getGroup()),
+              ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -1L);
         } catch (SchedulerException e) {
           LOGGER.error("Got exception when deleting the scheduled job - {}", jobKey, e);
         }
@@ -166,6 +160,10 @@ public class PinotTaskManager extends ControllerPeriodicTask<Void> {
     _tableTaskTypeToCronExpressionMap.remove(tableWithType);
   }
 
+  public static String getCronJobName(String tableWithType, String taskType) {
+    return String.format("%s.%s", tableWithType, taskType);
+  }
+
   public synchronized void subscribeTableConfigChanges(String tableWithType) {
     if (_tableTaskSchedulerUpdaterMap.containsKey(tableWithType)) {
       return;
@@ -220,6 +218,8 @@ public class PinotTaskManager extends ControllerPeriodicTask<Void> {
         if (!taskToCronExpressionMap.containsKey(existingTaskType)) {
           try {
             _scheduledExecutorService.deleteJob(JobKey.jobKey(tableWithType, existingTaskType));
+            _controllerMetrics.addMeteredTableValue(getCronJobName(tableWithType, existingTaskType),
+                ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED, -11L);
           } catch (SchedulerException e) {
             LOGGER.error("Failed to delete scheduled job for table {}, task type {}", tableWithType,
                 existingScheduledTasks, e);
@@ -290,6 +290,9 @@ public class PinotTaskManager extends ControllerPeriodicTask<Void> {
               .build();
       try {
         _scheduledExecutorService.scheduleJob(jobDetail, trigger);
+        _controllerMetrics
+            .addMeteredTableValue(getCronJobName(tableWithType, taskType), ControllerMeter.CRON_SCHEDULER_JOB_SCHEDULED,
+                1L);
       } catch (Exception e) {
         LOGGER.error("Failed to parse Cron expression - " + cronExprStr, e);
         throw e;
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java
index d2c2b13..9139f1d 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java
@@ -72,6 +72,10 @@ public abstract class ControllerPeriodicTask<C> extends BasePeriodicTask {
     }
   }
 
+  public final ControllerMetrics getControllerMetrics() {
+    return _controllerMetrics;
+  }
+
   /**
    * Processes the given list of tables, and returns the number of tables processed.
    * <p>


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