Kill all running tasks when the supervisor task is killed#7041
Kill all running tasks when the supervisor task is killed#7041jihoonson merged 3 commits intoapache:masterfrom
Conversation
| @Override | ||
| public TaskState run() throws Exception | ||
| { | ||
| if (baseFirehoseFactory.getNumSplits() == 0) { |
There was a problem hiding this comment.
wondering what if getNumSplits() returns 1
There was a problem hiding this comment.
If it's 1, the native parallel task currently does the same thing: the supervisor task will run a sub task for the single split. Do you think it's better to process in the supervisor task rather than processing in the sub task?
There was a problem hiding this comment.
may be i was thinking in that direction, but I guess it's okay to run a sub task for single split.
There was a problem hiding this comment.
Thanks. I think it makes sense, but probably better to fix in a separate PR.
| // Currently, this metric only represents # of killed tasks by ParallelIndexTaskRunner. | ||
| // See killAllRunningTasks(), SinglePhaseParallelIndexTaskRunner.run(), and | ||
| // SinglePhaseParallelIndexTaskRunner.stopGracefully() | ||
| @VisibleForTesting |
There was a problem hiding this comment.
I did not understand this annotation here, since numKilledTasks is private.
There was a problem hiding this comment.
Hmm.. I don't remember what I was thinking. Moved @ VisibleForTesting to getNumKilledTasks(). Thanks!
| } | ||
| } | ||
|
|
||
| int getNumKilledTasks() |
There was a problem hiding this comment.
May make sense to add the annotation @VisibleForTesting here instead
| @Override | ||
| public TaskState run() throws Exception | ||
| { | ||
| if (baseFirehoseFactory.getNumSplits() == 0) { |
There was a problem hiding this comment.
may be i was thinking in that direction, but I guess it's okay to run a sub task for single split.
| private int numFailedTasks; | ||
| // This metric is used only for unit tests because the current taskStatus system doesn't track the killed task status. | ||
| // Currently, this metric only represents # of killed tasks by ParallelIndexTaskRunner. | ||
| // See killAllRunningTasks(), SinglePhaseParallelIndexTaskRunner.run(), and |
There was a problem hiding this comment.
There was no method killAllRunningTasks() anywhere in the repo when this PR was checked in.
There was a problem hiding this comment.
Oops, thanks. Will fix it soon.
3 participants
Currently, the index_parallel supervisor task kills sub tasks when one of them fails, but it doesn't if someone kills itself. This PR is to kill sub tasks in
stopGracefully()of the supervisor task.