davidzollo commented on code in PR #17005:
URL:
https://github.com/apache/dolphinscheduler/pull/17005#discussion_r1958296863
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java:
##########
@@ -85,31 +85,105 @@ private ProcessUtils() {
private static final Pattern LINUXPATTERN =
Pattern.compile("\\((\\d+)\\)");
/**
- * kill tasks according to different task types.
+ * Terminate the task process, support multi-level signal processing and
fallback strategy
+ * @param request Task execution context
+ * @return Whether the process was successfully terminated
*/
- @Deprecated
public static boolean kill(@NonNull TaskExecutionContext request) {
try {
- log.info("Begin kill task instance, processId: {}",
request.getProcessId());
+ log.info("Begin killing task instance, processId: {}",
request.getProcessId());
int processId = request.getProcessId();
if (processId == 0) {
- log.error("Task instance kill failed, processId is not exist");
+ log.error("Task instance kill failed, processId is not
available");
return false;
}
- String cmd = String.format("kill -9 %s", getPidsStr(processId));
- cmd = OSUtils.getSudoCmd(request.getTenantCode(), cmd);
- log.info("process id:{}, cmd:{}", processId, cmd);
+ // Get all child processes
+ String pids = getPidsStr(processId);
+ String[] pidArray = pids.split("\\s+");
+ if (pidArray.length == 0) {
+ log.warn("No valid PIDs found for process: {}", processId);
+ return true;
+ }
+
+ // 1. Try to terminate gracefully (SIGINT)
+ boolean gracefulKillSuccess = sendKillSignal("SIGINT", pids,
request.getTenantCode(), 2000);
+ if (gracefulKillSuccess) {
+ log.info("Successfully killed process tree using SIGINT,
processId: {}", processId);
+ return true;
+ }
+
+ // 2. Try to terminate forcefully (SIGTERM)
+ boolean termKillSuccess = sendKillSignal("SIGTERM", pids,
request.getTenantCode(), 2000);
+ if (termKillSuccess) {
+ log.info("Successfully killed process tree using SIGTERM,
processId: {}", processId);
+ return true;
+ }
+
+ // 3. As a last resort, use `kill -9`
+ log.warn("SIGINT & SIGTERM failed, using SIGKILL as a last resort
for processId: {}", processId);
+ boolean forceKillSuccess = sendKillSignal("SIGKILL", pids,
request.getTenantCode(), 3000);
+ if (forceKillSuccess) {
+ log.info("Successfully killed process tree using SIGKILL,
processId: {}", processId);
+ return true;
+ }
- OSUtils.exeCmd(cmd);
- log.info("Success kill task instance, processId: {}",
request.getProcessId());
+ // 4. Finally, check if the process is still alive
+ for (String pid : pidArray) {
+ if (isProcessAlive(pid)) {
+ log.error("Failed to kill process {}, even after multiple
attempts", pid);
+ return false;
+ }
+ }
+
+ log.info("Success: process {} is no longer running", processId);
Review Comment:
You're right, it's no need to check here.
--
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]
