You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by js...@apache.org on 2019/09/03 04:59:19 UTC

[incubator-livy] branch master updated: [LIVY-519][TEST] Fix travis failed on should kill yarn app

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

jshao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-livy.git


The following commit(s) were added to refs/heads/master by this push:
     new 830d740  [LIVY-519][TEST] Fix travis failed on should kill yarn app
830d740 is described below

commit 830d740db5193314f469fa7bcbd4f6f93cbfc08b
Author: runzhiwang <ru...@tencent.com>
AuthorDate: Tue Sep 3 12:58:51 2019 +0800

    [LIVY-519][TEST] Fix travis failed on should kill yarn app
    
    ## What changes were proposed in this pull request?
    
    Fix travis failed on "should kill yarn app"
    
    The cause of failed is as follows:
    1. When create SparkYarnApp, the yarnAppMonitorThread will be created, which change app state to Failed. Because before recent commit https://github.com/apache/incubator-livy/commit/a90f4fac8be27a38cc961c24043a494a739ff188, the pair <RUNNING: getYarnApplicationState, SUCCEEDED: getFinalApplicationStatus> which was mocked in test, but was not defined in mapYarnState, so the state of app will be changed to failed.
    
    2. Then the test kills app, which will call killApplication when the app is running. However the app has been changed to failed in step 1, so killApplication won't be called, and verify(mockYarnClient).killApplication(appId) failed.
    
    3. So if yarnAppMonitorThread changes app state before main thread kills app, the test will failed. If not, the test will succeed.
    
    4. Though the recent commit https://github.com/apache/incubator-livy/commit/a90f4fac8be27a38cc961c24043a494a739ff188 fixed the bug accidentally, it is necessary to ensure the app is running before kill app.
    
    ## How was this patch tested?
    
    Existed UT and IT.
    
    Author: runzhiwang <ru...@tencent.com>
    
    Closes #221 from runzhiwang/LIVY-519.
---
 server/src/main/scala/org/apache/livy/utils/SparkYarnApp.scala     | 3 ++-
 server/src/test/scala/org/apache/livy/utils/SparkYarnAppSpec.scala | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/server/src/main/scala/org/apache/livy/utils/SparkYarnApp.scala b/server/src/main/scala/org/apache/livy/utils/SparkYarnApp.scala
index d51af62..14af9fa 100644
--- a/server/src/main/scala/org/apache/livy/utils/SparkYarnApp.scala
+++ b/server/src/main/scala/org/apache/livy/utils/SparkYarnApp.scala
@@ -209,7 +209,8 @@ class SparkYarnApp private[utils] (
       .getOrElse(IndexedSeq.empty)
   }
 
-  private def isRunning: Boolean = {
+  // Exposed for unit test.
+  private[utils] def isRunning: Boolean = {
     state != SparkApp.State.FAILED && state != SparkApp.State.FINISHED &&
       state != SparkApp.State.KILLED
   }
diff --git a/server/src/test/scala/org/apache/livy/utils/SparkYarnAppSpec.scala b/server/src/test/scala/org/apache/livy/utils/SparkYarnAppSpec.scala
index 823ae72..672444f 100644
--- a/server/src/test/scala/org/apache/livy/utils/SparkYarnAppSpec.scala
+++ b/server/src/test/scala/org/apache/livy/utils/SparkYarnAppSpec.scala
@@ -35,7 +35,7 @@ import org.mockito.stubbing.Answer
 import org.scalatest.FunSpec
 import org.scalatest.mock.MockitoSugar.mock
 
-import org.apache.livy.{LivyBaseUnitTestSuite, LivyConf}
+import org.apache.livy.{LivyBaseUnitTestSuite, LivyConf, Utils}
 import org.apache.livy.utils.SparkApp._
 
 class SparkYarnAppSpec extends FunSpec with LivyBaseUnitTestSuite {
@@ -145,6 +145,7 @@ class SparkYarnAppSpec extends FunSpec with LivyBaseUnitTestSuite {
         when(mockYarnClient.getApplicationReport(appId)).thenReturn(mockAppReport)
 
         val app = new SparkYarnApp(appTag, appIdOption, None, None, livyConf, mockYarnClient)
+        Utils.waitUntil({ () => app.isRunning }, Duration(10, TimeUnit.SECONDS))
         cleanupThread(app.yarnAppMonitorThread) {
           app.kill()
           appKilled = true