[GitHub] [incubator-kyuubi] zwangsheng commented on a diff in pull request #2314: [KYUUBI #2289] Use unique tag to kill applications

[GitBox <gi...@apache.org>]

zwangsheng commented on code in PR #2314:
URL: https://github.com/apache/incubator-kyuubi/pull/2314#discussion_r846961863


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala:
##########
@@ -193,28 +194,32 @@ class SparkProcessBuilder(
     }
   }
 
-  override def killApplication(line: String = lastRowsOfLog.toArray.mkString("\n")): String =
-    YARN_APP_NAME_REGEX.findFirstIn(line) match {
-      case Some(appId) =>
-        try {
-          val hadoopConf = KyuubiHadoopUtils.newHadoopConf(conf)
-          yarnClient.init(hadoopConf)
-          yarnClient.start()
-          val applicationId = ApplicationId.fromString(appId)
-          yarnClient.killApplication(applicationId)
-          s"Killed Application $appId successfully."
-        } catch {
-          case e: Throwable =>
-            s"Failed to kill Application $appId, please kill it manually." +
-              s" Caused by ${e.getMessage}."
-        } finally {
-          if (yarnClient != null) {
-            yarnClient.stop()
-          }
+  override def killApplication(engineRefId: String): String = {
+    val master = conf.getOption("spark.master").getOrElse("local")
+    if (master.equals("yarn")) {
+      var applicationId: ApplicationId = null
+      try {
+        val hadoopConf = KyuubiHadoopUtils.newHadoopConf(conf)
+        yarnClient.init(hadoopConf)
+        yarnClient.start()
+        val apps = yarnClient.getApplications(null, null, Set(engineRefId).asJava)
+        if (apps.isEmpty) return s"There are no Application tagged with $engineRefId"
+        applicationId = apps.asScala.head.getApplicationId
+        yarnClient.killApplication(applicationId)
+        s"Killed Application $applicationId successfully."
+      } catch {
+        case e: Throwable =>
+          s"Failed to kill Application $applicationId tagged with $engineRefId," +
+            s" please kill it manually. Caused by ${e.getMessage}."
+      } finally {
+        if (yarnClient != null) {
+          yarnClient.stop()
         }
-      case None => ""
+      }
+    } else {
+      ""

Review Comment:
   How about we add a warning here to tell that killApplication currently only works with YARN ?



-- 
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: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org