caishunfeng commented on code in PR #9624:
URL: https://github.com/apache/dolphinscheduler/pull/9624#discussion_r854003415
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/processor/TaskKillProcessor.java:
##########
@@ -146,27 +136,48 @@ private Pair<Boolean, List<String>>
doKill(TaskExecutionContext taskExecutionCon
}
/**
- * build TaskKillResponseCommand
- *
- * @param killCommand kill command
- * @param result exe result
- * @return build TaskKillResponseCommand
+ * kill task by cancel application
+ * @param taskInstanceId
*/
- private TaskKillResponseCommand
buildKillTaskResponseCommand(TaskKillRequestCommand killCommand,
- Pair<Boolean,
List<String>> result) {
- TaskKillResponseCommand taskKillResponseCommand = new
TaskKillResponseCommand();
- taskKillResponseCommand.setStatus(result.getLeft() ?
ExecutionStatus.SUCCESS.getCode() : ExecutionStatus.FAILURE.getCode());
- taskKillResponseCommand.setAppIds(result.getRight());
- TaskExecutionContext taskExecutionContext =
TaskExecutionContextCacheManager.getByTaskInstanceId(killCommand.getTaskInstanceId());
- if (taskExecutionContext == null) {
- return taskKillResponseCommand;
+ protected void cancelApplication(int taskInstanceId) {
+ TaskExecuteThread taskExecuteThread =
workerManager.getTaskExecuteThread(taskInstanceId);
+ if (taskExecuteThread == null) {
+ logger.warn("taskExecuteThread not found, taskInstanceId:{}",
taskInstanceId);
+ return;
+ }
+ AbstractTask task = taskExecuteThread.getTask();
+ if (task == null) {
+ logger.warn("task not found, taskInstanceId:{}", taskInstanceId);
+ return;
+ }
+ try {
+ task.cancelApplication(true);
Review Comment:
> I think that the `task cancelApplication` method and the following
`killProcess` method overlap. Is it necessary for the two methods to exist at
the same time?
Yes, but I think we should keep `killProcess` now if some task had not
impelemented `cancelApplication`, WDYT?
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/processor/TaskKillProcessor.java:
##########
@@ -122,21 +125,8 @@ public void process(Channel channel, Command command) {
*/
private Pair<Boolean, List<String>> doKill(TaskExecutionContext
taskExecutionContext) {
boolean processFlag = true;
- List<String> appIds = Collections.emptyList();
-
- try {
- String pidsStr =
ProcessUtils.getPidsStr(taskExecutionContext.getProcessId());
- if (!StringUtils.isEmpty(pidsStr)) {
- String cmd = String.format("kill -9 %s", pidsStr);
- cmd = OSUtils.getSudoCmd(taskExecutionContext.getTenantCode(),
cmd);
- logger.info("process id:{}, cmd:{}",
taskExecutionContext.getProcessId(), cmd);
- OSUtils.exeCmd(cmd);
- }
-
- } catch (Exception e) {
- processFlag = false;
Review Comment:
I will fix it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
