-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-28901][SQL] SparkThriftServer's Cancel SQL Operation show it in JDBC Tab UI #25611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 26 commits
b84d385
07d5679
187a4a6
44ba723
e8f902e
d4d1943
7d77b0c
72b885d
d2d6cc5
4c9d5f1
7b43b59
3744fc9
5070161
41ab7d7
87fa08f
63e8b59
bea260a
7e56c14
8b25006
ccd7de9
c6651f1
00f3553
8b84e04
ff5ac96
28174bd
1cbf7cc
61c9c73
f720963
c8d2ffc
536756b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,7 +138,7 @@ object HiveThriftServer2 extends Logging { | |
| } | ||
|
|
||
| private[thriftserver] object ExecutionState extends Enumeration { | ||
| val STARTED, COMPILED, FAILED, FINISHED, CLOSED = Value | ||
| val STARTED, COMPILED, CANCELED, FAILED, FINISHED, CLOSED = Value | ||
| type ExecutionState = Value | ||
| } | ||
|
|
||
|
|
@@ -174,16 +174,31 @@ object HiveThriftServer2 extends Logging { | |
| override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = { | ||
| server.stop() | ||
| } | ||
| private var onlineSessionNum: Int = 0 | ||
| private val sessionList = new mutable.LinkedHashMap[String, SessionInfo] | ||
| private val executionList = new mutable.LinkedHashMap[String, ExecutionInfo] | ||
| private val retainedStatements = conf.getConf(SQLConf.THRIFTSERVER_UI_STATEMENT_LIMIT) | ||
| private val retainedSessions = conf.getConf(SQLConf.THRIFTSERVER_UI_SESSION_LIMIT) | ||
| private var totalRunning = 0 | ||
|
|
||
| def getOnlineSessionNum: Int = synchronized { onlineSessionNum } | ||
| def getOnlineSessionNum: Int = synchronized { | ||
| sessionList.count(_._2.finishTimestamp == 0) | ||
| } | ||
|
|
||
| def isExecutionActive(execInfo: ExecutionInfo): Boolean = { | ||
| !(execInfo.state == ExecutionState.FAILED || | ||
| execInfo.state == ExecutionState.CANCELED || | ||
| execInfo.state == ExecutionState.CLOSED) | ||
| } | ||
|
|
||
| def getTotalRunning: Int = synchronized { totalRunning } | ||
| /** | ||
| * When an error or a cancellation occurs, we set the finishTimestamp of the statement. | ||
| * Therefore, when we count the number of running statements, we need to exclude errors and | ||
| * cancellations and count all statements that have not been closed so far. | ||
| */ | ||
| def getTotalRunning: Int = synchronized { | ||
| executionList.count { | ||
| case (_, v) => isExecutionActive(v) | ||
| } | ||
| } | ||
|
|
||
| def getSessionList: Seq[SessionInfo] = synchronized { sessionList.values.toSeq } | ||
|
|
||
|
|
@@ -208,14 +223,12 @@ object HiveThriftServer2 extends Logging { | |
| synchronized { | ||
| val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName) | ||
| sessionList.put(sessionId, info) | ||
| onlineSessionNum += 1 | ||
| trimSessionIfNecessary() | ||
| } | ||
| } | ||
|
|
||
| def onSessionClosed(sessionId: String): Unit = synchronized { | ||
| sessionList(sessionId).finishTimestamp = System.currentTimeMillis | ||
| onlineSessionNum -= 1 | ||
| trimSessionIfNecessary() | ||
| } | ||
|
|
||
|
|
@@ -231,28 +244,33 @@ object HiveThriftServer2 extends Logging { | |
| trimExecutionIfNecessary() | ||
| sessionList(sessionId).totalExecution += 1 | ||
| executionList(id).groupId = groupId | ||
| totalRunning += 1 | ||
| } | ||
|
|
||
| def onStatementParsed(id: String, executionPlan: String): Unit = synchronized { | ||
| executionList(id).executePlan = executionPlan | ||
| executionList(id).state = ExecutionState.COMPILED | ||
| } | ||
|
|
||
| def onStatementError(id: String, errorMessage: String, errorTrace: String): Unit = { | ||
| synchronized { | ||
| executionList(id).finishTimestamp = System.currentTimeMillis | ||
| executionList(id).detail = errorMessage | ||
| executionList(id).state = ExecutionState.FAILED | ||
| totalRunning -= 1 | ||
| trimExecutionIfNecessary() | ||
| } | ||
| def onStatementCanceled(id: String): Unit = synchronized { | ||
| executionList(id).finishTimestamp = System.currentTimeMillis | ||
| executionList(id).state = ExecutionState.CANCELED | ||
| trimExecutionIfNecessary() | ||
| } | ||
|
|
||
|
|
||
| def onStatementError( | ||
| id: String, | ||
| errorMessage: String, | ||
| errorTrace: String): Unit = synchronized { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ocd nit: changing |
||
| executionList(id).finishTimestamp = System.currentTimeMillis | ||
| executionList(id).detail = errorMessage | ||
| executionList(id).state = ExecutionState.FAILED | ||
| trimExecutionIfNecessary() | ||
| } | ||
|
|
||
| def onStatementFinish(id: String): Unit = synchronized { | ||
| executionList(id).finishTimestamp = System.currentTimeMillis | ||
| executionList(id).state = ExecutionState.FINISHED | ||
| totalRunning -= 1 | ||
| trimExecutionIfNecessary() | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,7 @@ private[hive] class SparkExecuteStatementOperation( | |
| runInBackground: Boolean = true) | ||
| (sqlContext: SQLContext, sessionToActivePool: JMap[SessionHandle, String]) | ||
| extends ExecuteStatementOperation(parentSession, statement, confOverlay, runInBackground) | ||
| with Logging { | ||
| with Logging { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: revert. https://github.com/databricks/scala-style-guide#spacing-and-indentation
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my local scala style may have some problem, not fit with that. |
||
|
|
||
| private var result: DataFrame = _ | ||
|
|
||
|
|
@@ -74,7 +74,6 @@ private[hive] class SparkExecuteStatementOperation( | |
| // RDDs will be cleaned automatically upon garbage collection. | ||
| logDebug(s"CLOSING $statementId") | ||
| cleanup(OperationState.CLOSED) | ||
| sqlContext.sparkContext.clearJobGroup() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: make that logDebug
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
maybe
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with both Close and Closed. I see that you already committed Closed. |
||
| HiveThriftServer2.listener.onOperationClosed(statementId) | ||
| } | ||
|
|
||
|
|
@@ -160,6 +159,14 @@ private[hive] class SparkExecuteStatementOperation( | |
| override def runInternal(): Unit = { | ||
| setState(OperationState.PENDING) | ||
| setHasResultSet(true) // avoid no resultset for async run | ||
| statementId = UUID.randomUUID().toString | ||
| logInfo(s"Submitting query '$statement' with $statementId") | ||
| HiveThriftServer2.listener.onStatementStart( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ocd formatting nit: move |
||
| statementId, | ||
| parentSession.getSessionHandle.getSessionId.toString, | ||
| statement, | ||
| statementId, | ||
| parentSession.getUsername) | ||
|
|
||
| if (!runInBackground) { | ||
| execute() | ||
|
|
@@ -201,33 +208,39 @@ private[hive] class SparkExecuteStatementOperation( | |
| setBackgroundHandle(backgroundHandle) | ||
| } catch { | ||
| case rejected: RejectedExecutionException => | ||
| logError("Error submit query in background, query rejected", rejected) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: "Error submit" -> "Error submitting" |
||
| setState(OperationState.ERROR) | ||
| HiveThriftServer2.listener.onStatementError( | ||
| statementId, rejected.getMessage, SparkUtils.exceptionString(rejected)) | ||
| throw new HiveSQLException("The background threadpool cannot accept" + | ||
| " new task for execution, please retry the operation", rejected) | ||
| case NonFatal(e) => | ||
| logError(s"Error executing query in background", e) | ||
| setState(OperationState.ERROR) | ||
| HiveThriftServer2.listener.onStatementError( | ||
| statementId, e.getMessage, SparkUtils.exceptionString(e)) | ||
| throw new HiveSQLException(e) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private def execute(): Unit = withSchedulerPool { | ||
| statementId = UUID.randomUUID().toString | ||
| logInfo(s"Running query '$statement' with $statementId") | ||
| setState(OperationState.RUNNING) | ||
| // Always use the latest class loader provided by executionHive's state. | ||
| val executionHiveClassLoader = sqlContext.sharedState.jarClassLoader | ||
| Thread.currentThread().setContextClassLoader(executionHiveClassLoader) | ||
|
|
||
| HiveThriftServer2.listener.onStatementStart( | ||
| statementId, | ||
| parentSession.getSessionHandle.getSessionId.toString, | ||
| statement, | ||
| statementId, | ||
| parentSession.getUsername) | ||
| sqlContext.sparkContext.setJobGroup(statementId, statement) | ||
| try { | ||
| synchronized { | ||
| if (getStatus.getState.isTerminal) { | ||
| logInfo(s"Query with $statementId in terminal state before it started running") | ||
| return | ||
| } else { | ||
| logInfo(s"Running query with $statementId") | ||
| setState(OperationState.RUNNING) | ||
| } | ||
| } | ||
| // Always use the latest class loader provided by executionHive's state. | ||
| val executionHiveClassLoader = sqlContext.sharedState.jarClassLoader | ||
| Thread.currentThread().setContextClassLoader(executionHiveClassLoader) | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @juliuszsompolski call cancel here for run statement in sync mode. seems can't stop task running . |
||
| sqlContext.sparkContext.setJobGroup(statementId, statement) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about move judgement to this place . I think
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AngersZhuuuu
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But when
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have
I think that the only way to prevent this, is to call another cancelJobGroup from the catch block when an exception comes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Get your point. |
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ocd nit: empty lines would be more readable if it was
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider a lot , learn it. |
||
| result = sqlContext.sql(statement) | ||
| logDebug(result.queryExecution.toString()) | ||
| result.queryExecution.logical match { | ||
|
|
@@ -249,32 +262,42 @@ private[hive] class SparkExecuteStatementOperation( | |
| } | ||
| dataTypes = result.queryExecution.analyzed.output.map(_.dataType).toArray | ||
| } catch { | ||
| case e: HiveSQLException => | ||
| if (getStatus().getState() == OperationState.CANCELED) { | ||
| // Actually do need to catch Throwable as some failures don't inherit from Exception and | ||
| // HiveServer will silently swallow them. | ||
| case e: Throwable => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AngersZhuuuu I think it can be fixed by:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got you point. But cancelJobGroup here seem can't stop |
||
| val currentState = getStatus().getState() | ||
| if (currentState.isTerminal) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ocd nit:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| // This may happen if the execution was cancelled, and then closed from another thread. | ||
| logWarning(s"Ignore exception in terminal state with $statementId: $e") | ||
| return | ||
|
AngersZhuuuu marked this conversation as resolved.
Outdated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: you either don't need the |
||
| } else { | ||
| logError(s"Error executing query with $statementId, currentState $currentState, ", e) | ||
| setState(OperationState.ERROR) | ||
| HiveThriftServer2.listener.onStatementError( | ||
| statementId, e.getMessage, SparkUtils.exceptionString(e)) | ||
| throw e | ||
| if (e.isInstanceOf[HiveSQLException]) { | ||
| throw e.asInstanceOf[HiveSQLException] | ||
| } else { | ||
| throw new HiveSQLException("Error running query: " + e.toString, e) | ||
| } | ||
| } | ||
| // Actually do need to catch Throwable as some failures don't inherit from Exception and | ||
| // HiveServer will silently swallow them. | ||
| case e: Throwable => | ||
| val currentState = getStatus().getState() | ||
| logError(s"Error executing query, currentState $currentState, ", e) | ||
| setState(OperationState.ERROR) | ||
| HiveThriftServer2.listener.onStatementError( | ||
| statementId, e.getMessage, SparkUtils.exceptionString(e)) | ||
| throw new HiveSQLException(e.toString) | ||
| } finally { | ||
| synchronized { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. collateral fix: please move
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Miss this point , fix it. |
||
| setState(OperationState.FINISHED) | ||
| HiveThriftServer2.listener.onStatementFinish(statementId) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missed it: You still need to wrap it in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add if here is the most direct way, otherwise add too much control |
||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think this could become a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Reasonable, I add too much control. |
||
| sqlContext.sparkContext.clearJobGroup() | ||
| } | ||
| setState(OperationState.FINISHED) | ||
| HiveThriftServer2.listener.onStatementFinish(statementId) | ||
| } | ||
|
|
||
| override def cancel(): Unit = { | ||
| logInfo(s"Cancel '$statement' with $statementId") | ||
| cleanup(OperationState.CANCELED) | ||
| synchronized { | ||
| if (!getStatus.getState.isTerminal) { | ||
| logInfo(s"Cancel query with $statementId") | ||
| cleanup(OperationState.CANCELED) | ||
| HiveThriftServer2.listener.onStatementCanceled(statementId) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private def cleanup(state: OperationState) { | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
promise final nit: remove double empty line