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 2022/04/07 20:39:11 UTC
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8486: Add endpoints for some finer control on minion tasks
Jackie-Jiang commented on code in PR #8486:
URL: https://github.com/apache/pinot/pull/8486#discussion_r845531899
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -213,6 +224,16 @@ public StringResultResponse getTaskStateDeprecated(
return _pinotHelixTaskResourceManager.getTaskConfigs(taskName);
}
+ @GET
+ @Path("/tasks/subtask/{taskName}/config")
+ @ApiOperation("Get the task state for the given task")
Review Comment:
Update the doc
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -196,6 +199,14 @@ public TaskState getTaskState(
return _pinotHelixTaskResourceManager.getTaskState(taskName);
}
+ @GET
+ @Path("/tasks/subtask/{taskName}/state")
+ @ApiOperation("Get the task state for the given task")
Review Comment:
Update the doc
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -162,6 +164,26 @@ public synchronized void deleteTaskQueue(String taskType, boolean forceDelete) {
_taskDriver.delete(helixJobQueueName, forceDelete);
}
+ /**
+ * Delete tasks from the task queue for the given task type.
+ *
+ * @param taskType to find the task queue.
+ * @param taskNames a list of tasks to delete from the queue.
+ * @param forceDelete as said in helix comment, if set true, all job's related zk nodes will
+ * be clean up from zookeeper even if its workflow information can not be found.
+ */
+ public synchronized void deleteTasks(String taskType, String taskNames, boolean forceDelete) {
+ String helixJobQueueName = getHelixJobQueueName(taskType);
+ for (String taskName : StringUtils.split(taskNames, ",")) {
Review Comment:
```suggestion
for (String taskName : StringUtils.split(taskNames, ',')) {
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -344,6 +392,32 @@ public synchronized TaskState getTaskState(String taskName) {
return taskConfigs;
}
+ /**
+ * Get the configs of specified sub task for the given task name.
+ * @param taskName the task whose sub tasks to check
+ * @param subTaskNames the sub tasks to check
+ * @return the configs of the sub tasks
+ */
+ public synchronized Map<String, PinotTaskConfig> getSubTaskConfigs(String taskName, String subTaskNames) {
+ JobConfig jobConfig = _taskDriver.getJobConfig(getHelixJobName(taskName));
+ if (jobConfig == null) {
+ return Collections.emptyMap();
+ }
+ Map<String, TaskConfig> helixTaskConfigs = jobConfig.getTaskConfigMap();
+ Map<String, PinotTaskConfig> taskConfigs = new HashMap<>(helixTaskConfigs.size());
+ if (StringUtils.isEmpty(subTaskNames)) {
+ helixTaskConfigs.forEach((sub, cfg) -> taskConfigs.put(sub, PinotTaskConfig.fromHelixTaskConfig(cfg)));
+ return taskConfigs;
+ }
+ for (String subTaskName : StringUtils.split(subTaskNames, ",")) {
Review Comment:
```suggestion
for (String subTaskName : StringUtils.split(subTaskNames, ',')) {
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -419,12 +440,17 @@ public SuccessResponse toggleTaskQueueState(
@DELETE
@Path("/tasks/{taskType}")
@Authenticate(AccessType.DELETE)
- @ApiOperation("Delete all tasks (as well as the task queue) for the given task type")
+ @ApiOperation("Delete specified tasks or all the tasks (as well as the task queue) for the given task type")
Review Comment:
I'd suggest adding a separate API for deleting task `DELETE /tasks/task`.
Do we need to support deleting multiple tasks, or always one at a time? Deleting multiple tasks might fail in the middle and left half of the tasks.
IIRC, in order to delete a task, we need to first stop the task queue?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -344,6 +392,32 @@ public synchronized TaskState getTaskState(String taskName) {
return taskConfigs;
}
+ /**
+ * Get the configs of specified sub task for the given task name.
+ * @param taskName the task whose sub tasks to check
+ * @param subTaskNames the sub tasks to check
+ * @return the configs of the sub tasks
+ */
+ public synchronized Map<String, PinotTaskConfig> getSubTaskConfigs(String taskName, String subTaskNames) {
Review Comment:
```suggestion
public synchronized Map<String, PinotTaskConfig> getSubTaskConfigs(String taskName, @Nullable String subTaskNames) {
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -196,6 +199,14 @@ public TaskState getTaskState(
return _pinotHelixTaskResourceManager.getTaskState(taskName);
}
+ @GET
Review Comment:
(minor) Move this after the `getTaskStateDeprecated`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java:
##########
@@ -213,6 +224,16 @@ public StringResultResponse getTaskStateDeprecated(
return _pinotHelixTaskResourceManager.getTaskConfigs(taskName);
}
+ @GET
+ @Path("/tasks/subtask/{taskName}/config")
+ @ApiOperation("Get the task state for the given task")
+ public Map<String, PinotTaskConfig> getSubTaskConfigs(
Review Comment:
(minor) We treat `subtask` as one word. Same for other places
```suggestion
public Map<String, PinotTaskConfig> getSubtaskConfigs(
```
--
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-unsubscribe@pinot.apache.org
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