You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by fe...@apache.org on 2022/12/09 08:27:34 UTC

[incubator-kyuubi] branch branch-1.6 updated (4f14a8484 -> 97247e6d0)

This is an automated email from the ASF dual-hosted git repository.

feiwang pushed a change to branch branch-1.6
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


 discard 4f14a8484 [KYUUBI #3922] Only the ApplicationInfo with non-empty id is valid for BatchJobSubmission
     new 97247e6d0 [KYUUBI #3922] Only the ApplicationInfo with non-empty id is valid for BatchJobSubmission

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (4f14a8484)
            \
             N -- N -- N   refs/heads/branch-1.6 (97247e6d0)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala      | 1 +
 .../scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala   | 5 -----
 2 files changed, 1 insertion(+), 5 deletions(-)


[incubator-kyuubi] 01/01: [KYUUBI #3922] Only the ApplicationInfo with non-empty id is valid for BatchJobSubmission

Posted by fe...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

feiwang pushed a commit to branch branch-1.6
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git

commit 97247e6d0c50ed48911ca5df3257e26720e8dd4c
Author: fwang12 <fw...@ebay.com>
AuthorDate: Fri Dec 9 00:21:54 2022 +0800

    [KYUUBI #3922] Only the ApplicationInfo with non-empty id is valid for BatchJobSubmission
    
    ![image](https://user-images.githubusercontent.com/6757692/206124635-2e43542a-337f-455b-bccc-f7907d5e26db.png)
    
    The batch state is update to running, but app state is NOT_FOUND.
    
    We shall check the ApplicationInfo return by applicationManager.
    
    Because for YarnApplicationOperation, the result of getApplicationInfoByTag is always non-empty.
    ```
          if (reports.isEmpty) {
            debug(s"Application with tag $tag not found")
            ApplicationInfo(id = null, name = null, state = ApplicationState.NOT_FOUND)
          }
    ```
    
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [x] Add screenshots for manual tests if appropriate
    
    - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #3922 from turboFei/not_found.
    
    Closes #3922
    
    f83c67c2 [fwang12] set not found state if finally state is empty
    62e619b0 [fwang12] fix ut
    53c76d29 [fwang12] fix ut
    ff5c36b3 [fwang12] check id none empty
    7b1180c9 [fwang12] add ut
    4843f8a7 [fwang12] do not update not found as batch state
    
    Authored-by: fwang12 <fw...@ebay.com>
    Signed-off-by: fwang12 <fw...@ebay.com>
    (cherry picked from commit 8eac51385b42c42a29d34e2208d512a7bc2d86bf)
    Signed-off-by: fwang12 <fw...@ebay.com>
---
 .../scala/org/apache/kyuubi/operation/BatchJobSubmission.scala | 10 +++++++++-
 .../test/scala/org/apache/kyuubi/RestClientTestHelper.scala    |  1 +
 .../test/scala/org/apache/kyuubi/WithKyuubiServerOnYarn.scala  |  9 +++++++--
 .../org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala |  2 +-
 .../org/apache/kyuubi/server/rest/client/BatchCliSuite.scala   |  8 ++++----
 5 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
index bcaca6237..6936cd5ed 100644
--- a/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
+++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/operation/BatchJobSubmission.scala
@@ -99,7 +99,8 @@ class BatchJobSubmission(
   }
 
   private[kyuubi] def currentApplicationInfo: Option[ApplicationInfo] = {
-    applicationManager.getApplicationInfo(builder.clusterManager(), batchId)
+    // only the ApplicationInfo with non-empty id is valid for the operation
+    applicationManager.getApplicationInfo(builder.clusterManager(), batchId).filter(_.id != null)
   }
 
   private[kyuubi] def killBatchApplication(): KillResponse = {
@@ -117,6 +118,13 @@ class BatchJobSubmission(
         0L
       }
 
+    if (isTerminalState(state)) {
+      if (applicationInfo.isEmpty) {
+        applicationInfo =
+          Option(ApplicationInfo(id = null, name = null, state = ApplicationState.NOT_FOUND))
+      }
+    }
+
     applicationInfo.foreach { status =>
       val metadataToUpdate = Metadata(
         identifier = batchId,
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala
index d15993797..f3d11432d 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/RestClientTestHelper.scala
@@ -56,6 +56,7 @@ trait RestClientTestHelper extends RestFrontendTestHelper with KerberizedTestHel
       .set(
         KyuubiConf.AUTHENTICATION_CUSTOM_CLASS,
         classOf[UserDefineAuthenticationProviderImpl].getCanonicalName)
+      .set(KyuubiConf.BATCH_APPLICATION_CHECK_INTERVAL, 100)
   }
 
 }
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/WithKyuubiServerOnYarn.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/WithKyuubiServerOnYarn.scala
index cb55450ad..727c5545e 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/WithKyuubiServerOnYarn.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/WithKyuubiServerOnYarn.scala
@@ -122,6 +122,12 @@ class KyuubiOperationYarnClusterSuite extends WithKyuubiServerOnYarn with HiveJD
       assert(appInfo.exists(_.id.startsWith("application_")))
     }
 
+    eventually(timeout(10.seconds)) {
+      val metadata = session.sessionManager.getBatchMetadata(session.handle.identifier.toString)
+      assert(metadata.state === "RUNNING")
+      assert(metadata.engineId.startsWith("application_"))
+    }
+
     val killResponse = yarnOperation.killApplicationByTag(sessionHandle.identifier.toString)
     assert(killResponse._1)
     assert(killResponse._2 startsWith "Succeeded to terminate:")
@@ -169,8 +175,7 @@ class KyuubiOperationYarnClusterSuite extends WithKyuubiServerOnYarn with HiveJD
     val batchJobSubmissionOp = session.batchJobSubmissionOp
 
     eventually(timeout(3.minutes), interval(50.milliseconds)) {
-      assert(batchJobSubmissionOp.currentApplicationInfo.isDefined)
-      assert(batchJobSubmissionOp.currentApplicationInfo.get.id == null)
+      assert(batchJobSubmissionOp.currentApplicationInfo.isEmpty)
       assert(batchJobSubmissionOp.getStatus.state === OperationState.ERROR)
     }
   }
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
index efc53375d..05d70d411 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/BatchesResourceSuite.scala
@@ -436,7 +436,7 @@ class BatchesResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper wi
     assert(session1.createTime === batchMetadata.createTime)
     assert(session2.createTime === batchMetadata2.createTime)
 
-    eventually(timeout(5.seconds)) {
+    eventually(timeout(10.seconds)) {
       assert(session1.batchJobSubmissionOp.getStatus.state === OperationState.RUNNING ||
         session1.batchJobSubmissionOp.getStatus.state === OperationState.FINISHED)
       assert(session1.batchJobSubmissionOp.builder.processLaunched)
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
index ff52bf7fb..18e583e9a 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchCliSuite.scala
@@ -28,7 +28,7 @@ import org.apache.hive.service.rpc.thrift.TProtocolVersion
 import org.scalatest.time.SpanSugar.convertIntToGrainOfTime
 
 import org.apache.kyuubi.{BatchTestHelper, RestClientTestHelper, Utils}
-import org.apache.kyuubi.ctl.TestPrematureExit
+import org.apache.kyuubi.ctl.{CtlConf, TestPrematureExit}
 import org.apache.kyuubi.metrics.{MetricsConstants, MetricsSystem}
 import org.apache.kyuubi.session.KyuubiSessionManager
 
@@ -234,12 +234,12 @@ class BatchCliSuite extends RestClientTestHelper with TestPrematureExit with Bat
       "--password",
       ldapUserPasswd,
       "--waitCompletion",
-      "false")
+      "false",
+      "--conf",
+      s"${CtlConf.CTL_BATCH_LOG_QUERY_INTERVAL.key}=100")
     val result = testPrematureExitForControlCli(submitArgs, "")
     assert(result.contains(s"/bin/spark-submit"))
     assert(!result.contains("ShutdownHookManager: Shutdown hook called"))
-    val numberOfRows = result.split("\n").length
-    assert(numberOfRows <= 100)
   }
 
   test("list batch test") {