Jackie-Jiang commented on a change in pull request #7300:
URL: https://github.com/apache/pinot/pull/7300#discussion_r688735058
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -452,6 +640,25 @@ public void addToError(int error) {
_error += error;
}
+ public void addToCompleted(int completed) {
Review comment:
We can remove all these `addTo*` and only allow `addTaskState`
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -427,10 +491,134 @@ private static String getTaskType(String name) {
return name.split(TASK_NAME_SEPARATOR)[1];
}
+ public static class TaskDebugInfo {
+ private String _startTime = null;
+ private String _executionStartTime = null;
+ private TaskState _taskState = null;
+ private TaskCount _subTaskCounts = null;
+ private List<TaskPartitionDebugInfo> _subTaskInfos = null;
+
+ public TaskDebugInfo() {
+ }
+
+ public void setStartTime(String startTime) {
+ _startTime = startTime;
+ }
+
+ public void setExecutionStartTime(String executionStartTime) {
+ _executionStartTime = executionStartTime;
+ }
+
+ public void setTaskState(TaskState taskState) {
+ _taskState = taskState;
+ }
+
+ public void setSubTaskCount(TaskCount subTaskCount) {
+ _subTaskCounts = subTaskCount;
+ }
+
+ public void addSubTaskInfo(TaskPartitionDebugInfo subTaskInfo) {
+ if (_subTaskInfos == null) {
+ _subTaskInfos = new ArrayList<>();
+ }
+ _subTaskInfos.add(subTaskInfo);
+ }
+
+ public String getStartTime() {
+ return _startTime;
+ }
+
+ public String getExecutionStartTime() {
+ return _executionStartTime;
+ }
+
+ public TaskState getTaskState() {
+ return _taskState;
+ }
+
+ public TaskCount getSubTaskCount() {
+ return _subTaskCounts;
+ }
+
+ public List<TaskPartitionDebugInfo> getSubTaskInfos() {
+ return _subTaskInfos;
+ }
+ }
+
+ public static class TaskPartitionDebugInfo {
+ private String _taskId = null;
+ private TaskPartitionState _state = null;
+ private String _startTime = null;
+ private String _finishTime = null;
+ private String _participant = null;
+ private String _info = null;
+ private PinotTaskConfig _taskConfig = null;
+
+ public TaskPartitionDebugInfo() {
+ }
+
+ public void setTaskId(String taskId) {
+ _taskId = taskId;
+ }
+
+ public void setState(TaskPartitionState state) {
+ _state = state;
+ }
+
+ public void setStartTime(String startTime) {
+ _startTime = startTime;
+ }
+
+ public void setFinishTime(String finishTime) {
+ _finishTime = finishTime;
+ }
+
+ public void setParticipant(String participant) {
+ _participant = participant;
+ }
+
+ public void setInfo(String info) {
+ _info = info;
+ }
+
+ public void setTaskConfig(PinotTaskConfig taskConfig) {
+ _taskConfig = taskConfig;
+ }
+
+ public String getTaskId() {
+ return _taskId;
+ }
+
+ public TaskPartitionState getState() {
+ return _state;
+ }
+
+ public String getStartTime() {
+ return _startTime;
+ }
+
+ public String getFinishTime() {
+ return _finishTime;
+ }
+
+ public String getParticipant() {
+ return _participant;
+ }
+
+ public String getInfo() {
+ return _info;
+ }
+
+ public PinotTaskConfig getTaskConfig() {
+ return _taskConfig;
+ }
+ }
+
public static class TaskCount {
Review comment:
Let's fix the order of these fields by putting a `JsonPropertyOrder`
annotation
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
Review comment:
(nit) Don't define it here
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
+ SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd
HH:mm:ss z", Locale.getDefault());
+ SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
+ Map<String, TaskState> helixJobStates = workflowContext.getJobStates();
+ for (String helixJobName : helixJobStates.keySet()) {
+ String pinotTaskName = getPinotTaskName(helixJobName);
+ TaskDebugInfo taskDebugInfo = new TaskDebugInfo();
+ taskDebugInfo.setTaskState(workflowContext.getJobState(helixJobName));
+ timeMillis = workflowContext.getJobStartTime(helixJobName);
+ taskDebugInfo.setStartTime((timeMillis <= 0) ? null :
SIMPLE_DATE_FORMAT.format(timeMillis));
+ JobContext jobContext = _taskDriver.getJobContext(helixJobName);
+ if (jobContext != null) {
+ JobConfig jobConfig = _taskDriver.getJobConfig(helixJobName);
+ timeMillis = jobContext.getExecutionStartTime();
+ taskDebugInfo.setExecutionStartTime((timeMillis <= 0) ? null :
SIMPLE_DATE_FORMAT.format(timeMillis));
+ Set<Integer> partitionSet = jobContext.getPartitionSet();
+ TaskCount subTaskCount = new TaskCount();
+ subTaskCount.addToTotal(partitionSet.size());
+ for (int partition : partitionSet) {
Review comment:
(Optional) We can avoid a lot of redundant map lookups by directly
accessing the `jobContext.getRecord().getMapFields().entrySet()`. You may read
the `JobContext` code to understand how each field is extracted
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -452,6 +640,25 @@ public void addToError(int error) {
_error += error;
}
+ public void addToCompleted(int completed) {
+ _completed += completed;
+ }
+
+ /* Update count based on state for each task running under a
HelixJob/PinotTask */
+ public void addTaskState(TaskPartitionState state) {
+ // Helix returns state as null if the task is not enqueued anywhere yet
+ if (state == null) {
+ // task is not yet assigned to a participant
+ addToWaiting(1);
+ } else if (state.equals(TaskPartitionState.INIT) ||
state.equals(TaskPartitionState.RUNNING)) {
+ addToRunning(1);
+ } else if (state.equals(TaskPartitionState.TASK_ERROR)) {
+ addToError(1);
+ } else if (state.equals(TaskPartitionState.COMPLETED)) {
+ addToCompleted(1);
+ }
Review comment:
Let's add a catch all state (e.g. `Unknown`) in case we got other states
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
+ SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd
HH:mm:ss z", Locale.getDefault());
Review comment:
We should make this a util to avoid different class using different
format. We can do it in a separate PR
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
+ SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd
HH:mm:ss z", Locale.getDefault());
+ SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
+ Map<String, TaskState> helixJobStates = workflowContext.getJobStates();
+ for (String helixJobName : helixJobStates.keySet()) {
Review comment:
Use `entrySet()` to avoid another lookup to get the job state
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
Review comment:
Why do we choose >= 10 instead of > 1?
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
##########
@@ -131,6 +131,15 @@ public StringResultResponse getTaskQueueStateDeprecated(
return _pinotHelixTaskResourceManager.getTaskStatesByTable(taskType,
tableNameWithType);
}
+ @GET
+ @Path("/tasks/{taskType}/debug")
+ @ApiOperation("Fetch information for all the tasks for the given task type")
+ public Map<String, PinotHelixTaskResourceManager.TaskDebugInfo>
getTaskDebugInfo(
+ @ApiParam(value = "Task type", required = true) @PathParam("taskType")
String taskType,
+ @ApiParam(value = "verbosity (default prints for running and error
tasks. Value of >=10 prints for all tasks)") @DefaultValue("1")
@QueryParam("verboseLevel") int verboseLevel) {
Review comment:
Let's use `verbosity` to be consistent with the segment debug info
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -427,10 +491,134 @@ private static String getTaskType(String name) {
return name.split(TASK_NAME_SEPARATOR)[1];
}
+ public static class TaskDebugInfo {
Review comment:
Let's fix the order of these fields by putting a `JsonPropertyOrder`
annotation
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
+ SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd
HH:mm:ss z", Locale.getDefault());
+ SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
+ Map<String, TaskState> helixJobStates = workflowContext.getJobStates();
+ for (String helixJobName : helixJobStates.keySet()) {
+ String pinotTaskName = getPinotTaskName(helixJobName);
+ TaskDebugInfo taskDebugInfo = new TaskDebugInfo();
+ taskDebugInfo.setTaskState(workflowContext.getJobState(helixJobName));
+ timeMillis = workflowContext.getJobStartTime(helixJobName);
+ taskDebugInfo.setStartTime((timeMillis <= 0) ? null :
SIMPLE_DATE_FORMAT.format(timeMillis));
+ JobContext jobContext = _taskDriver.getJobContext(helixJobName);
+ if (jobContext != null) {
+ JobConfig jobConfig = _taskDriver.getJobConfig(helixJobName);
+ timeMillis = jobContext.getExecutionStartTime();
+ taskDebugInfo.setExecutionStartTime((timeMillis <= 0) ? null :
SIMPLE_DATE_FORMAT.format(timeMillis));
+ Set<Integer> partitionSet = jobContext.getPartitionSet();
+ TaskCount subTaskCount = new TaskCount();
+ subTaskCount.addToTotal(partitionSet.size());
+ for (int partition : partitionSet) {
+ // First get the partition's state and update the subTaskCount
+ TaskPartitionState partitionState =
jobContext.getPartitionState(partition);
+ subTaskCount.addTaskState(partitionState);
+ if (!showCompleted) {
+ // Skip details for COMPLETED tasks
+ if (partitionState != null &&
partitionState.equals(TaskPartitionState.COMPLETED)) {
+ continue;
+ }
+ }
Review comment:
```suggestion
// Skip details for COMPLETED tasks
if (!showCompleted && partitionState ==
TaskPartitionState.COMPLETED) {
continue;
}
```
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -427,10 +491,134 @@ private static String getTaskType(String name) {
return name.split(TASK_NAME_SEPARATOR)[1];
}
+ public static class TaskDebugInfo {
+ private String _startTime = null;
+ private String _executionStartTime = null;
+ private TaskState _taskState = null;
+ private TaskCount _subTaskCounts = null;
+ private List<TaskPartitionDebugInfo> _subTaskInfos = null;
+
+ public TaskDebugInfo() {
+ }
+
+ public void setStartTime(String startTime) {
+ _startTime = startTime;
+ }
+
+ public void setExecutionStartTime(String executionStartTime) {
+ _executionStartTime = executionStartTime;
+ }
+
+ public void setTaskState(TaskState taskState) {
+ _taskState = taskState;
+ }
+
+ public void setSubTaskCount(TaskCount subTaskCount) {
+ _subTaskCounts = subTaskCount;
+ }
+
+ public void addSubTaskInfo(TaskPartitionDebugInfo subTaskInfo) {
+ if (_subTaskInfos == null) {
+ _subTaskInfos = new ArrayList<>();
+ }
+ _subTaskInfos.add(subTaskInfo);
+ }
+
+ public String getStartTime() {
+ return _startTime;
+ }
+
+ public String getExecutionStartTime() {
+ return _executionStartTime;
+ }
+
+ public TaskState getTaskState() {
+ return _taskState;
+ }
+
+ public TaskCount getSubTaskCount() {
+ return _subTaskCounts;
+ }
+
+ public List<TaskPartitionDebugInfo> getSubTaskInfos() {
+ return _subTaskInfos;
+ }
+ }
+
+ public static class TaskPartitionDebugInfo {
+ private String _taskId = null;
Review comment:
(nit) no need to explicitly put `= null` for member variables
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
Review comment:
Use a `TreeMap` so that the entries are ordered by the task name
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java
##########
@@ -381,6 +377,74 @@ public synchronized TaskState getTaskState(String
taskName) {
return filteredTaskStateMap;
}
+ /**
+ * Given a taskType, helper method to debug all the HelixJobs for the
taskType.
+ * For each of the HelixJobs, collects status of the (sub)tasks in the
taskbatch.
+ *
+ * @param taskType Pinot taskType / Helix JobQueue
+ * @param verboseLevel By default, does not show details for completed
tasks.
+ * If verboseLevel >= 10, shows details for all tasks.
+ * @return Map of Pinot Task Name to TaskDebugInfo. TaskDebugInfo contains
details for subtasks.
+ */
+ public synchronized Map<String, TaskDebugInfo> getTaskDebugInfo(String
taskType, int verboseLevel) {
+ Map<String, TaskDebugInfo> taskDebugInfos = new HashMap<>();
+ WorkflowContext workflowContext =
_taskDriver.getWorkflowContext(getHelixJobQueueName(taskType));
+ if (workflowContext == null) {
+ return taskDebugInfos;
+ }
+ boolean showCompleted = verboseLevel >= 10;
+ long timeMillis;
+ SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd
HH:mm:ss z", Locale.getDefault());
+ SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
+ Map<String, TaskState> helixJobStates = workflowContext.getJobStates();
+ for (String helixJobName : helixJobStates.keySet()) {
+ String pinotTaskName = getPinotTaskName(helixJobName);
+ TaskDebugInfo taskDebugInfo = new TaskDebugInfo();
+ taskDebugInfo.setTaskState(workflowContext.getJobState(helixJobName));
+ timeMillis = workflowContext.getJobStartTime(helixJobName);
+ taskDebugInfo.setStartTime((timeMillis <= 0) ? null :
SIMPLE_DATE_FORMAT.format(timeMillis));
Review comment:
(nit) for readability. Same for other time handling
```suggestion
long jobStartTimeMs = workflowContext.getJobStartTime(helixJobName);
if (jobStartTimeMs > 0) {
taskDebugInfo.setStartTime(SIMPLE_DATE_FORMAT.format(jobStartTimeMs));
}
```
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
