This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 700cffe800 Fix up parsing of controller timers
tableRebalanceExecutionTimeMs and cronSchedulerJobExecutionTimeMs (#15330)
700cffe800 is described below
commit 700cffe8001c8a19c370f07f815226c2aaeb6799
Author: Sonam Mandal <sonam.man...@startree.ai>
AuthorDate: Wed Mar 26 14:46:11 2025 -0700
Fix up parsing of controller timers tableRebalanceExecutionTimeMs and
cronSchedulerJobExecutionTimeMs (#15330)
---
.../configs/controller.yml | 17 ++++++++++++++
.../ControllerPrometheusMetricsTest.java | 27 +++++++++++++++++-----
.../prometheus/PinotPrometheusMetricsTest.java | 16 +++++++++----
3 files changed, 50 insertions(+), 10 deletions(-)
diff --git
a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml
b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml
index a1b945ab4a..4b06ad572e 100644
--- a/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml
+++ b/docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml
@@ -48,6 +48,23 @@ rules:
table: "$2$4"
tableType: "$5"
taskType: "$6"
+ # Special handling for timers like cronSchedulerJobExecutionTimeMs and
tableRebalanceExecutionTimeMs which use table name, table type and another
string for status / taskType
+- pattern:
"\"org\\.apache\\.pinot\\.common\\.metrics\"<type=\"ControllerMetrics\",
name=\"pinot\\.controller\\.(([^.]+)\\.)?([^.]*)_(OFFLINE|REALTIME)\\.(\\w+)\\.cronSchedulerJobExecutionTimeMs\"><>(\\w+)"
+ name: "pinot_controller_cronSchedulerJobExecutionTimeMs_$6"
+ cache: true
+ labels:
+ database: "$2"
+ table: "$1$3"
+ tableType: "$4"
+ taskType: "$5"
+- pattern:
"\"org\\.apache\\.pinot\\.common\\.metrics\"<type=\"ControllerMetrics\",
name=\"pinot\\.controller\\.(([^.]+)\\.)?([^.]*)_(OFFLINE|REALTIME)\\.(\\w+)\\.tableRebalanceExecutionTimeMs\"><>(\\w+)"
+ name: "pinot_controller_tableRebalanceExecutionTimeMs_$6"
+ cache: true
+ labels:
+ database: "$2"
+ table: "$1$3"
+ tableType: "$4"
+ status: "$5"
# Gauges that accept taskType and task status
- pattern:
"\"org\\.apache\\.pinot\\.common\\.metrics\"<type=\"ControllerMetrics\",
name=\"pinot\\.controller\\.([^.]*)\\.([^.]*)\\.(\\w+)\"><>(\\w+)"
name: "pinot_controller_$1_$4"
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/ControllerPrometheusMetricsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/ControllerPrometheusMetricsTest.java
index 1061a0af54..45583184d4 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/ControllerPrometheusMetricsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/ControllerPrometheusMetricsTest.java
@@ -72,13 +72,28 @@ public abstract class ControllerPrometheusMetricsTest
extends PinotPrometheusMet
_controllerMetrics.addTimedValue(controllerTimer, 30_000,
TimeUnit.MILLISECONDS);
assertTimerExportedCorrectly(controllerTimer.getTimerName(),
EXPORTED_METRIC_PREFIX);
} else {
- _controllerMetrics.addTimedTableValue(TABLE_NAME_WITH_TYPE,
controllerTimer, 30_000L, TimeUnit.MILLISECONDS);
- _controllerMetrics.addTimedTableValue(ExportedLabelValues.TABLENAME,
controllerTimer, 30_000L,
- TimeUnit.MILLISECONDS);
+ if (controllerTimer ==
ControllerTimer.TABLE_REBALANCE_EXECUTION_TIME_MS) {
+ _controllerMetrics.addTimedTableValue(String.format("%s.%s",
TABLE_NAME_WITH_TYPE, ExportedLabelValues.DONE
- assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.TABLENAME_TABLETYPE,
- EXPORTED_METRIC_PREFIX);
- assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.TABLENAME, EXPORTED_METRIC_PREFIX);
+ ),
+ ControllerTimer.TABLE_REBALANCE_EXECUTION_TIME_MS, 100_000,
TimeUnit.MILLISECONDS);
+ assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.JOBSTATUS_TABLENAME_TABLETYPE,
+ EXPORTED_METRIC_PREFIX);
+ } else if (controllerTimer ==
ControllerTimer.CRON_SCHEDULER_JOB_EXECUTION_TIME_MS) {
+ _controllerMetrics.addTimedTableValue(String.format("%s.%s",
TABLE_NAME_WITH_TYPE,
+ ExportedLabelValues.MINION_TASK_SEGMENT_IMPORT),
+ ControllerTimer.CRON_SCHEDULER_JOB_EXECUTION_TIME_MS, 100_000,
TimeUnit.MILLISECONDS);
+ assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.TASKTYPE_TABLENAME_TABLETYPE,
+ EXPORTED_METRIC_PREFIX);
+ } else {
+ _controllerMetrics.addTimedTableValue(TABLE_NAME_WITH_TYPE,
controllerTimer, 30_000L, TimeUnit.MILLISECONDS);
+ _controllerMetrics.addTimedTableValue(ExportedLabelValues.TABLENAME,
controllerTimer, 30_000L,
+ TimeUnit.MILLISECONDS);
+
+ assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.TABLENAME_TABLETYPE,
+ EXPORTED_METRIC_PREFIX);
+ assertTimerExportedCorrectly(controllerTimer.getTimerName(),
ExportedLabels.TABLENAME, EXPORTED_METRIC_PREFIX);
+ }
}
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/PinotPrometheusMetricsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/PinotPrometheusMetricsTest.java
index 2de1ce8c8b..5173bc00df 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/PinotPrometheusMetricsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/metrics/prometheus/PinotPrometheusMetricsTest.java
@@ -165,8 +165,8 @@ public abstract class PinotPrometheusMetricsTest {
List<PromMetric> promMetrics;
try {
promMetrics =
parseExportedPromMetrics(getExportedPromMetrics().getResponse());
- for (String meterType : TIMER_TYPES) {
- PromMetric expectedTimer = PromMetric.withName(exportedMetricPrefix +
exportedTimerPrefix + "_" + meterType);
+ for (String timerType : TIMER_TYPES) {
+ PromMetric expectedTimer = PromMetric.withName(exportedMetricPrefix +
exportedTimerPrefix + "_" + timerType);
Assert.assertTrue(promMetrics.contains(expectedTimer),
"Cannot find timer: " + expectedTimer + " in exported metrics");
}
@@ -180,9 +180,9 @@ public abstract class PinotPrometheusMetricsTest {
List<PromMetric> promMetrics;
try {
promMetrics =
parseExportedPromMetrics(getExportedPromMetrics().getResponse());
- for (String meterType : METER_TYPES) {
+ for (String timerType : TIMER_TYPES) {
PromMetric expectedTimer =
- PromMetric.withNameAndLabels(exportedMetricPrefix +
exportedTimerPrefix + "_" + meterType, labels);
+ PromMetric.withNameAndLabels(exportedMetricPrefix +
exportedTimerPrefix + "_" + timerType, labels);
Assert.assertTrue(promMetrics.contains(expectedTimer),
"Cannot find timer: " + expectedTimer + " in exported metrics");
}
@@ -329,6 +329,13 @@ public abstract class PinotPrometheusMetricsTest {
List.of(ExportedLabelKeys.TABLE, ExportedLabelValues.TABLENAME,
ExportedLabelKeys.TABLETYPE,
ExportedLabelValues.TABLETYPE_REALTIME, ExportedLabelKeys.TASKTYPE,
ExportedLabelValues.MINION_TASK_SEGMENT_IMPORT);
+
+ public static final List<String> JOBSTATUS_TABLENAME_TABLETYPE =
+ List.of(STATUS, ExportedLabelValues.DONE, TABLE,
ExportedLabelValues.TABLENAME, TABLETYPE, TABLETYPE_REALTIME);
+
+ public static final List<String> TASKTYPE_TABLENAME_TABLETYPE =
+ List.of(TASKTYPE, ExportedLabelValues.MINION_TASK_SEGMENT_IMPORT,
TABLE, ExportedLabelValues.TABLENAME,
+ TABLETYPE, TABLETYPE_REALTIME);
}
public static class ExportedLabelKeys {
@@ -351,6 +358,7 @@ public abstract class PinotPrometheusMetricsTest {
public static final String CONTROLLER_PERIODIC_TASK_CHC =
"ClusterHealthCheck";
public static final String MINION_TASK_SEGMENT_IMPORT =
"SegmentImportTask";
public static final String IN_PROGRESS = "IN_PROGRESS";
+ public static final String DONE = "DONE";
}
/*
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
