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:15:33 UTC
[GitHub] [pinot] klsince opened a new pull request, #8486: Add endpoints for some finer control on minion tasks
klsince opened a new pull request, #8486:
URL: https://github.com/apache/pinot/pull/8486
add endpoints for some finer control on minion tasks:
- inspect the states or configs of the subtasks of a task
- allow to delete individual tasks w/o deleting the task queue
## Description
<!-- Add a description of your PR here.
A good description should include pointers to an issue or design document, etc.
-->
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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
[GitHub] [pinot] klsince commented on a diff in pull request #8486: Add endpoints for some finer control on minion tasks
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8486:
URL: https://github.com/apache/pinot/pull/8486#discussion_r845584439
##########
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:
correct, deleting a job requires to stop the task queue firstly; otherwise, the method throws exception.
yeah, I can start with deleting one task a time for simplicity, and that'd be enough for most use cases today, because many of our minion tasks stop scheduling new tasks if there is one still running.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8486: Add endpoints for some finer control on minion tasks
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [pinot] codecov-commenter commented on pull request #8486: Add endpoints for some finer control on minion tasks
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8486:
URL: https://github.com/apache/pinot/pull/8486#issuecomment-1092276640
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8486](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0f19bbd) into [master](https://codecov.io/gh/apache/pinot/commit/6849441b5b676e1b2685bd4335093f140d423c5b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6849441) will **decrease** coverage by `1.95%`.
> The diff coverage is `12.50%`.
> :exclamation: Current head 0f19bbd differs from pull request most recent head a890ce6. Consider uploading reports for the commit a890ce6 to get more accurate results
```diff
@@ Coverage Diff @@
## master #8486 +/- ##
==========================================
- Coverage 29.15% 27.19% -1.96%
==========================================
Files 1660 1660
Lines 87082 87122 +40
Branches 13194 13202 +8
==========================================
- Hits 25386 23691 -1695
- Misses 59367 61201 +1834
+ Partials 2329 2230 -99
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `27.19% <12.50%> (+0.12%)` | :arrow_up: |
| integration2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...roller/api/resources/PinotTaskRestletResource.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFza1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `40.47% <13.88%> (-3.71%)` | :arrow_down: |
| [.../apache/pinot/server/access/RequesterIdentity.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9SZXF1ZXN0ZXJJZGVudGl0eS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/plan/StreamingInstanceResponsePlanNode.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ0luc3RhbmNlUmVzcG9uc2VQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ore/operator/streaming/StreamingResponseUtils.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nUmVzcG9uc2VVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ager/realtime/PeerSchemeSplitSegmentCommitter.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUGVlclNjaGVtZVNwbGl0U2VnbWVudENvbW1pdHRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/broker/broker/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| [...instanceselector/ReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1JlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (-90.00%)` | :arrow_down: |
| [...he/pinot/core/plan/StreamingSelectionPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL1N0cmVhbWluZ1NlbGVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
| ... and [167 more](https://codecov.io/gh/apache/pinot/pull/8486/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6849441...a890ce6](https://codecov.io/gh/apache/pinot/pull/8486?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [pinot] Jackie-Jiang merged pull request #8486: Add endpoints for some finer control on minion tasks
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #8486:
URL: https://github.com/apache/pinot/pull/8486
--
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