Radeity commented on code in PR #13939:
URL: 
https://github.com/apache/dolphinscheduler/pull/13939#discussion_r1168553987


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/LogUtils.java:
##########
@@ -183,6 +183,17 @@ public List<String> getAppIdsFromLogFile(@NonNull String 
logPath) {
         }
     }
 
+    public String getAppIdsFromLogLine(String logLine) {
+        if (StringUtils.isBlank(logLine))
+            return null;
+        Matcher matcher = APPLICATION_REGEX.matcher(logLine);
+        if (matcher.find()) {
+            String appId = matcher.group();
+            return appId;
+        }
+        return null;
+    }
+

Review Comment:
   Same, we don't need this. When we kill a DataxTask, we will find 
applicationId in log file if it is submitted to Yarn.



##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/am/YarnApplicationManager.java:
##########
@@ -160,8 +160,8 @@ private String getJobHistoryUrl(String applicationId) {
      * @param commandFile command file
      * @param cmd cmd
      */
-    private void execYarnKillCommand(String tenantCode, String appId, String 
commandFile,
-                                     String cmd) {
+    public static void execYarnKillCommand(String tenantCode, String appId, 
String commandFile,

Review Comment:
   You can remove `static`.



##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java:
##########
@@ -306,6 +312,12 @@ public void cancelApplication() throws 
InterruptedException {
             return;
         }
 
+        if (StringUtils.isNotBlank(appId)) {
+            String commandFile = String.format("%s/%s.kill", 
taskRequest.getExecutePath(), appId);
+            
YarnApplicationManager.execYarnKillCommand(taskRequest.getTenantCode(), appId, 
commandFile,
+                    "yarn application -kill " + appId);
+        }
+

Review Comment:
   We don't have to kill yarn application here, like other yarn tasks(Spark, 
Flink, etc), plz remove code changes in `AbstractCommandExecutor`.



-- 
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]

Reply via email to