Skip to content

Fix minion wait &run time alerts to self-resolve using gauge #18517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
swaminathanmanish wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix minion wait &run time alerts to self-resolve using gauge #18517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37f51de
Fix MinionSubTaskHighWaitTime alert to self-resolve using gauge
swaminathanmanish May 18, 2026
2e72de7
Remove unused SUBTASK_WAITING_TIME timer — replaced by MAX_SUBTASK_WA…
swaminathanmanish May 26, 2026
b155ab8
Replace SUBTASK_RUNNING_TIME timer with MAX_SUBTASK_RUNNING_TIME_MS g…
swaminathanmanish May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public enum ControllerGauge implements AbstractMetrics.Gauge {
// TODO: Unify below subtask metrics into a single metric with status label
NUM_MINION_TASKS_IN_PROGRESS("NumMinionTasksInProgress", true),
NUM_MINION_SUBTASKS_WAITING("NumMinionSubtasksWaiting", true),
MAX_SUBTASK_WAIT_TIME_MS("MaxSubtaskWaitTimeMs", false),
NUM_MINION_SUBTASKS_RUNNING("NumMinionSubtasksRunning", true),
MAX_SUBTASK_RUNNING_TIME_MS("MaxSubtaskRunningTimeMs", false),
NUM_MINION_SUBTASKS_ERROR("NumMinionSubtasksError", true),
NUM_MINION_SUBTASKS_UNKNOWN("NumMinionSubtasksUnknown", true),
NUM_MINION_SUBTASKS_DROPPED("NumMinionSubtasksDropped", true),
Expand Down
5 changes: 1 addition & 4 deletions pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerTimer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ public enum ControllerTimer implements AbstractMetrics.Timer {
DEEP_STORE_SEGMENT_WRITE_TIME_MS("deepStoreSegmentWriteTimeMs", true),
// Audit logging timers
AUDIT_REQUEST_PROCESSING_TIME("auditRequestProcessingTime", true),
AUDIT_RESPONSE_PROCESSING_TIME("auditResponseProcessingTime", true),
// Log subtask waiting (until not started) and running (until not completed) time
SUBTASK_WAITING_TIME("subtaskWaitingTime", false),
SUBTASK_RUNNING_TIME("subtaskRunningTime", false);
AUDIT_RESPONSE_PROCESSING_TIME("auditResponseProcessingTime", true);


private final String _timerName;
Expand Down
22 changes: 16 additions & 6 deletions ...oller/src/main/java/org/apache/pinot/controller/helix/core/minion/TaskMetricsEmitter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.apache.pinot.common.metrics.ControllerGauge;
import org.apache.pinot.common.metrics.ControllerMetrics;
import org.apache.pinot.common.metrics.ControllerTimer;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.LeadControllerManager;
import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
Expand Down Expand Up @@ -109,6 +107,8 @@ protected final void runTask(Properties periodicTaskProperties) {
for (String taskType : taskTypes) {
TaskCount taskTypeAccumulatedCount = new TaskCount();
Map<String, TaskCount> tableAccumulatedCount = new HashMap<>();
Map<String, Long> tableMaxWaitTimeMs = new HashMap<>();
Map<String, Long> tableMaxRunningTimeMs = new HashMap<>();
try {
// Capture the current execution timestamp for this task type collection cycle
long currentExecutionTimestamp = System.currentTimeMillis();
Expand Down Expand Up @@ -147,12 +147,10 @@ protected final void runTask(Properties periodicTaskProperties) {
}
count.accumulate(taskCount);
taskStatusSummary.getSubtaskWaitingTimes().values().forEach(subtaskWaitingTime -> {
_controllerMetrics.addTimedTableValue(tableNameWithType, ControllerTimer.SUBTASK_WAITING_TIME,
subtaskWaitingTime, TimeUnit.MILLISECONDS);
tableMaxWaitTimeMs.merge(tableNameWithType, subtaskWaitingTime, Math::max);
});
taskStatusSummary.getSubtaskRunningTimes().values().forEach(subtaskRunningTime -> {
_controllerMetrics.addTimedTableValue(tableNameWithType, ControllerTimer.SUBTASK_RUNNING_TIME,
subtaskRunningTime, TimeUnit.MILLISECONDS);
tableMaxRunningTimeMs.merge(tableNameWithType, subtaskRunningTime, Math::max);
});
return count;
});
Expand Down Expand Up @@ -211,6 +209,16 @@ protected final void runTask(Properties periodicTaskProperties) {
ControllerGauge.PERCENT_MINION_SUBTASKS_IN_ERROR, tablePercent);
});

// Emit 0 for tables with no waiting/running subtasks so the gauge (and alert) self-resolves
tableAccumulatedCount.keySet().forEach(tableNameWithType -> {
_controllerMetrics.setOrUpdateTableGauge(tableNameWithType, taskType,
ControllerGauge.MAX_SUBTASK_WAIT_TIME_MS,
tableMaxWaitTimeMs.getOrDefault(tableNameWithType, 0L));
_controllerMetrics.setOrUpdateTableGauge(tableNameWithType, taskType,
ControllerGauge.MAX_SUBTASK_RUNNING_TIME_MS,
tableMaxRunningTimeMs.getOrDefault(tableNameWithType, 0L));
});

if (_preReportedTables.containsKey(taskType)) {
Set<String> tableNameWithTypeSet = _preReportedTables.get(taskType);
tableNameWithTypeSet.removeAll(tableAccumulatedCount.keySet());
Expand Down Expand Up @@ -286,6 +294,8 @@ private void removeTableTaskTypeMetrics(Set<String> tableNameWithTypeSet, String
ControllerGauge.PERCENT_MINION_SUBTASKS_IN_QUEUE);
_controllerMetrics.removeTableGauge(tableNameWithType, taskType,
ControllerGauge.PERCENT_MINION_SUBTASKS_IN_ERROR);
_controllerMetrics.removeTableGauge(tableNameWithType, taskType, ControllerGauge.MAX_SUBTASK_WAIT_TIME_MS);
_controllerMetrics.removeTableGauge(tableNameWithType, taskType, ControllerGauge.MAX_SUBTASK_RUNNING_TIME_MS);
});
}
}
31 changes: 26 additions & 5 deletions ...r/src/test/java/org/apache/pinot/controller/helix/core/minion/TaskMetricsEmitterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,9 @@ public void taskType1WithTwoTablesEmitMetricTwice() {
private void runAndAssertForTaskType1WithTwoTables() {
PinotMetricsRegistry metricsRegistry = _controllerMetrics.getMetricsRegistry();
_taskMetricsEmitter.runTask(null);
// Expected 31 metrics: 29 original + 2 timing metrics (SUBTASK_WAITING_TIME and SUBTASK_RUNNING_TIME)
Assert.assertEquals(metricsRegistry.allMetrics().size(), 31);
// Expected 33 metrics: 29 original + MAX_SUBTASK_WAIT_TIME_MS gauge (x2 tables)
// + MAX_SUBTASK_RUNNING_TIME_MS gauge (x2 tables)
Assert.assertEquals(metricsRegistry.allMetrics().size(), 33);

Assert.assertTrue(metricsRegistry.allMetrics().containsKey(
new YammerMetricName(ControllerMetrics.class, "pinot.controller.onlineMinionInstances")));
Expand Down Expand Up @@ -250,6 +251,26 @@ private void runAndAssertForTaskType1WithTwoTables() {
new YammerMetricName(ControllerMetrics.class,
"pinot.controller.percentMinionSubtasksInError.table2_OFFLINE.taskType1"))
.getMetric()).value(), 50L);

// table1 has a waiting subtask (subtask2, 3000ms); table2 has none (0ms)
Assert.assertEquals(((YammerSettableGauge<?>) metricsRegistry.allMetrics().get(
new YammerMetricName(ControllerMetrics.class,
"pinot.controller.maxSubtaskWaitTimeMs.table1_OFFLINE.taskType1"))
.getMetric()).value(), 3000L);
Assert.assertEquals(((YammerSettableGauge<?>) metricsRegistry.allMetrics().get(
new YammerMetricName(ControllerMetrics.class,
"pinot.controller.maxSubtaskWaitTimeMs.table2_OFFLINE.taskType1"))
.getMetric()).value(), 0L);

// table2 has a running subtask (subtask1, 5000ms); table1 has none (0ms)
Assert.assertEquals(((YammerSettableGauge<?>) metricsRegistry.allMetrics().get(
new YammerMetricName(ControllerMetrics.class,
"pinot.controller.maxSubtaskRunningTimeMs.table1_OFFLINE.taskType1"))
.getMetric()).value(), 0L);
Assert.assertEquals(((YammerSettableGauge<?>) metricsRegistry.allMetrics().get(
new YammerMetricName(ControllerMetrics.class,
"pinot.controller.maxSubtaskRunningTimeMs.table2_OFFLINE.taskType1"))
.getMetric()).value(), 5000L);
}

@Test
Expand Down Expand Up @@ -281,9 +302,9 @@ private void oneTaskTypeWithOneTable(String taskType, String taskName1, String t

PinotMetricsRegistry metricsRegistry = _controllerMetrics.getMetricsRegistry();
_taskMetricsEmitter.runTask(null);
// Expected at least 21 metrics: 20 original + 1 timing metric (SUBTASK_WAITING_TIME)
// The actual count may vary slightly based on test execution order
Assert.assertTrue(metricsRegistry.allMetrics().size() >= 21);
// Expected at least 22 metrics: 20 original + MAX_SUBTASK_WAIT_TIME_MS gauge (x1 table)
// + MAX_SUBTASK_RUNNING_TIME_MS gauge (x1 table). Count may vary based on test execution order.
Assert.assertTrue(metricsRegistry.allMetrics().size() >= 22);

Assert.assertTrue(metricsRegistry.allMetrics().containsKey(
new YammerMetricName(ControllerMetrics.class, "pinot.controller.onlineMinionInstances")));
Expand Down
Loading