Repository: incubator-livy Updated Branches: refs/heads/master cd777e209 -> 1efc80180
[LIVY-415][CORE][FOLLOWUP] Fix static object creation issue for SessionState ## What changes were proposed in this pull request? In LIVY-415, we changed `SessionState` to static object instead of case class. This is OK for `SessionState`, but for `FinishedSessionState`, it requires `time` field to be set during object creation. Using static object will never reflect the actual `time` when object is created. So here propose to fix it. CC aa8y please take a review, this is introduced in your changes. ## How was this patch tested? Existing tests. Author: jerryshao <ss...@hortonworks.com> Closes #67 from jerryshao/LIVY-415-followup. Project: http://git-wip-us.apache.org/repos/asf/incubator-livy/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-livy/commit/1efc8018 Tree: http://git-wip-us.apache.org/repos/asf/incubator-livy/tree/1efc8018 Diff: http://git-wip-us.apache.org/repos/asf/incubator-livy/diff/1efc8018 Branch: refs/heads/master Commit: 1efc801806619d7741f453eff07afac33e841d7a Parents: cd777e2 Author: jerryshao <ss...@hortonworks.com> Authored: Wed Nov 29 12:49:39 2017 +0800 Committer: jerryshao <ss...@hortonworks.com> Committed: Wed Nov 29 12:49:39 2017 +0800 ---------------------------------------------------------------------- .../org/apache/livy/sessions/SessionState.scala | 15 ++++++--------- .../apache/livy/test/framework/LivyRestClient.scala | 6 +++--- .../main/scala/org/apache/livy/repl/Session.scala | 4 ++-- .../org/apache/livy/server/batch/BatchSession.scala | 4 ++-- .../livy/server/interactive/InteractiveSession.scala | 8 ++++---- .../apache/livy/sessions/SessionManagerSpec.scala | 6 +++--- 6 files changed, 20 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/core/src/main/scala/org/apache/livy/sessions/SessionState.scala ---------------------------------------------------------------------- diff --git a/core/src/main/scala/org/apache/livy/sessions/SessionState.scala b/core/src/main/scala/org/apache/livy/sessions/SessionState.scala index 577a27a..e56cfbb 100644 --- a/core/src/main/scala/org/apache/livy/sessions/SessionState.scala +++ b/core/src/main/scala/org/apache/livy/sessions/SessionState.scala @@ -37,9 +37,9 @@ object SessionState { case "running" => Running case "busy" => Busy case "shutting_down" => ShuttingDown - case "error" => Error - case "dead" => Dead - case "success" => Success + case "error" => Error() + case "dead" => Dead() + case "success" => Success() case _ => throw new IllegalArgumentException(s"Illegal session state: $s") } @@ -57,15 +57,12 @@ object SessionState { object ShuttingDown extends SessionState("shutting_down", false) - case class Error(override val time: Long) extends + case class Error(override val time: Long = System.nanoTime()) extends FinishedSessionState("error", true, time) - object Error extends Error(System.nanoTime) - case class Dead(override val time: Long) extends + case class Dead(override val time: Long = System.nanoTime()) extends FinishedSessionState("dead", false, time) - object Dead extends Dead(System.nanoTime) - case class Success(override val time: Long) extends + case class Success(override val time: Long = System.nanoTime()) extends FinishedSessionState("success", false, time) - object Success extends Success(System.nanoTime) } http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/integration-test/src/main/scala/org/apache/livy/test/framework/LivyRestClient.scala ---------------------------------------------------------------------- diff --git a/integration-test/src/main/scala/org/apache/livy/test/framework/LivyRestClient.scala b/integration-test/src/main/scala/org/apache/livy/test/framework/LivyRestClient.scala index 941a650..f22221f 100644 --- a/integration-test/src/main/scala/org/apache/livy/test/framework/LivyRestClient.scala +++ b/integration-test/src/main/scala/org/apache/livy/test/framework/LivyRestClient.scala @@ -111,9 +111,9 @@ class LivyRestClient(val httpClient: AsyncHttpClient, val livyEndpoint: String) } class BatchSession(id: Int) extends Session(id, BATCH_TYPE) { - def verifySessionDead(): Unit = verifySessionState(SessionState.Dead) + def verifySessionDead(): Unit = verifySessionState(SessionState.Dead()) def verifySessionRunning(): Unit = verifySessionState(SessionState.Running) - def verifySessionSuccess(): Unit = verifySessionState(SessionState.Success) + def verifySessionSuccess(): Unit = verifySessionState(SessionState.Success()) } class InteractiveSession(id: Int) extends Session(id, INTERACTIVE_TYPE) { @@ -226,7 +226,7 @@ class LivyRestClient(val httpClient: AsyncHttpClient, val livyEndpoint: String) .setBody(mapper.writeValueAsString(requestBody)) .execute() - verifySessionState(SessionState.Dead) + verifySessionState(SessionState.Dead()) } def verifySessionIdle(): Unit = { http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/repl/src/main/scala/org/apache/livy/repl/Session.scala ---------------------------------------------------------------------- diff --git a/repl/src/main/scala/org/apache/livy/repl/Session.scala b/repl/src/main/scala/org/apache/livy/repl/Session.scala index 5d471cd..09e4f6c 100644 --- a/repl/src/main/scala/org/apache/livy/repl/Session.scala +++ b/repl/src/main/scala/org/apache/livy/repl/Session.scala @@ -136,7 +136,7 @@ class Session( entries }(interpreterExecutor) - future.onFailure { case _ => changeState(SessionState.Error) }(interpreterExecutor) + future.onFailure { case _ => changeState(SessionState.Error()) }(interpreterExecutor) future } @@ -297,7 +297,7 @@ class Session( (TRACEBACK -> traceback) case Interpreter.ExecuteAborted(message) => - changeState(SessionState.Error) + changeState(SessionState.Error()) (STATUS -> ERROR) ~ (EXECUTION_COUNT -> executionCount) ~ http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/server/src/main/scala/org/apache/livy/server/batch/BatchSession.scala ---------------------------------------------------------------------- diff --git a/server/src/main/scala/org/apache/livy/server/batch/BatchSession.scala b/server/src/main/scala/org/apache/livy/server/batch/BatchSession.scala index ae9b212..c5b4b23 100644 --- a/server/src/main/scala/org/apache/livy/server/batch/BatchSession.scala +++ b/server/src/main/scala/org/apache/livy/server/batch/BatchSession.scala @@ -178,9 +178,9 @@ class BatchSession( _state = SessionState.Running info(s"Batch session $id created [appid: ${appId.orNull}, state: ${state.toString}, " + s"info: ${appInfo.asJavaMap}]") - case SparkApp.State.FINISHED => _state = SessionState.Success + case SparkApp.State.FINISHED => _state = SessionState.Success() case SparkApp.State.KILLED | SparkApp.State.FAILED => - _state = SessionState.Dead + _state = SessionState.Dead() case _ => } } http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSession.scala ---------------------------------------------------------------------- diff --git a/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSession.scala b/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSession.scala index 26c8d93..d12fcb3 100644 --- a/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSession.scala +++ b/server/src/main/scala/org/apache/livy/server/interactive/InteractiveSession.scala @@ -398,7 +398,7 @@ class InteractiveSession( } if (client.isEmpty) { - transition(Dead) + transition(Dead()) val msg = s"Cannot recover interactive session $id because its RSCDriver URI is unknown." info(msg) sessionLog = IndexedSeq(msg) @@ -435,7 +435,7 @@ class InteractiveSession( // this callback might be triggered. Check and don't call stop() to avoid nested called // if the session is already shutting down. if (serverSideState != SessionState.ShuttingDown) { - transition(SessionState.Error) + transition(SessionState.Error()) stop() app.foreach { a => info(s"Failed to ping RSC driver for session $id. Killing application.") @@ -474,7 +474,7 @@ class InteractiveSession( _.kill() } } finally { - transition(SessionState.Dead) + transition(SessionState.Dead()) } } @@ -611,7 +611,7 @@ class InteractiveSession( debug(s"$this app state changed from $oldState to $newState") newState match { case SparkApp.State.FINISHED | SparkApp.State.KILLED | SparkApp.State.FAILED => - transition(SessionState.Dead) + transition(SessionState.Dead()) case _ => } } http://git-wip-us.apache.org/repos/asf/incubator-livy/blob/1efc8018/server/src/test/scala/org/apache/livy/sessions/SessionManagerSpec.scala ---------------------------------------------------------------------- diff --git a/server/src/test/scala/org/apache/livy/sessions/SessionManagerSpec.scala b/server/src/test/scala/org/apache/livy/sessions/SessionManagerSpec.scala index 69ec817..547af8b 100644 --- a/server/src/test/scala/org/apache/livy/sessions/SessionManagerSpec.scala +++ b/server/src/test/scala/org/apache/livy/sessions/SessionManagerSpec.scala @@ -98,9 +98,9 @@ class SessionManagerSpec extends FunSpec with Matchers with LivyBaseUnitTestSuit } // Stopped session should be gc-ed after retained timeout - for (s <- Seq(SessionState.Error, - SessionState.Success, - SessionState.Dead)) { + for (s <- Seq(SessionState.Error(), + SessionState.Success(), + SessionState.Dead())) { eventually(timeout(30 seconds), interval(100 millis)) { changeStateAndCheck(s) { sm => sm.get(session.id) should be (None) } }
