Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -932,11 +932,20 @@ object SQLConf {
"a positive value, a running query will be cancelled automatically when the timeout is " +
"exceeded, otherwise the query continues to run till completion. If timeout values are " +
"set for each statement via `java.sql.Statement.setQueryTimeout` and they are smaller " +
"than this configuration value, they take precedence.")
"than this configuration value, they take precedence. After set this config, you may be " +
"interesting in spark.sql.thriftServer.interruptOnCancel which can help interrupt task.")
Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
.version("3.1.0")
.timeConf(TimeUnit.SECONDS)
.createWithDefault(0L)

val THRIFTSERVER_FORCE_CANCEL =
buildConf("spark.sql.thriftServer.interruptOnCancel")
.doc("When true, all running tasks will be interrupted if one cancels a query. " +
"When false, all running tasks will be remained until finished.")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we guide users to enable this configuration when spark.sql.thriftServer.queryTimeout is set?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah It sounds so warm !

Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
.version("3.2.0")
.booleanConf
.createWithDefault(false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware that cancelling a job group leaves currently running tasks running to completion.
Why not set it true? What are the disadvantages?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is same with Why the default behavior of sc.cancelJob does not interrupt task, related JIRA SPARK-17064. And also I think the default value set to false can keep the same behavior with early version.


val THRIFTSERVER_UI_STATEMENT_LIMIT =
buildConf("spark.sql.thriftserver.ui.retainedStatements")
.doc("The number of SQL statements kept in the JDBC/ODBC web UI history.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ private[hive] class SparkExecuteStatementOperation(
}
}

private val forceCancel = sqlContext.conf.getConf(SQLConf.THRIFTSERVER_FORCE_CANCEL)

private val substitutorStatement = SQLConf.withExistingConf(sqlContext.conf) {
new VariableSubstitution().substitute(statement)
}
Expand Down Expand Up @@ -131,7 +133,7 @@ private[hive] class SparkExecuteStatementOperation(

def getNextRowSet(order: FetchOrientation, maxRowsL: Long): RowSet = withLocalProperties {
try {
sqlContext.sparkContext.setJobGroup(statementId, substitutorStatement)
sqlContext.sparkContext.setJobGroup(statementId, substitutorStatement, forceCancel)
getNextRowSetInternal(order, maxRowsL)
} finally {
sqlContext.sparkContext.clearJobGroup()
Expand Down Expand Up @@ -321,7 +323,7 @@ private[hive] class SparkExecuteStatementOperation(
parentSession.getSessionState.getConf.setClassLoader(executionHiveClassLoader)
}

sqlContext.sparkContext.setJobGroup(statementId, substitutorStatement)
sqlContext.sparkContext.setJobGroup(statementId, substitutorStatement, forceCancel)
result = sqlContext.sql(statement)
logDebug(result.queryExecution.toString())
HiveThriftServer2.eventManager.onStatementParsed(statementId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@
package org.apache.spark.sql.hive.thriftserver

import java.sql.SQLException
import java.util.concurrent.atomic.AtomicBoolean

import org.apache.hive.service.cli.HiveSQLException

import org.apache.spark.TaskKilled
import org.apache.spark.scheduler.{SparkListener, SparkListenerTaskEnd}
import org.apache.spark.sql.internal.SQLConf

trait ThriftServerWithSparkContextSuite extends SharedThriftServer {

test("the scratch dir will be deleted during server start but recreated with new operation") {
Expand Down Expand Up @@ -79,6 +84,38 @@ trait ThriftServerWithSparkContextSuite extends SharedThriftServer {
"java.lang.NumberFormatException: invalid input syntax for type numeric: 1.2"))
}
}

test("SPARK-33526: Add config to control if cancel invoke interrupt task on thriftserver") {
withJdbcStatement { statement =>
val forceCancel = new AtomicBoolean(false)
val listener = new SparkListener {
override def onTaskEnd(taskEnd: SparkListenerTaskEnd): Unit = {
assert(taskEnd.reason.isInstanceOf[TaskKilled])
if (forceCancel.get()) {
assert(System.currentTimeMillis() - taskEnd.taskInfo.launchTime < 1000)
} else {
// avoid accuracy, we check 2s instead of 3s.
assert(System.currentTimeMillis() - taskEnd.taskInfo.launchTime >= 2000)
}
}
}

spark.sparkContext.addSparkListener(listener)
try {
statement.execute(s"SET ${SQLConf.THRIFTSERVER_QUERY_TIMEOUT.key}=1")
Seq(true, false).foreach { force =>
statement.execute(s"SET ${SQLConf.THRIFTSERVER_FORCE_CANCEL.key}=$force")
forceCancel.set(force)
val e1 = intercept[SQLException] {
statement.execute("select java_method('java.lang.Thread', 'sleep', 3000L)")
}.getMessage
assert(e1.contains("Query timed out"))
}
} finally {
spark.sparkContext.removeSparkListener(listener)
}
}
}
}


Expand Down