Skip to content
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

[SPARK-19043][SQL]Make SparkSQLSessionManager more configurable #16445

Closed
wants to merge 1 commit into from
Closed

[SPARK-19043][SQL]Make SparkSQLSessionManager more configurable #16445

wants to merge 1 commit into from

Conversation

yaooqinn
Copy link
Member

What changes were proposed in this pull request?

To make SparkSQLSessionManager's background operation thread pool not only configurable for pool size but also the wait queue size and the thread keep alive time. To be consistent with HiveServer2's behavior.

How was this patch tested?

manual tests

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@@ -34,28 +30,13 @@ import org.apache.spark.sql.hive.thriftserver.server.SparkSQLOperationManager


private[hive] class SparkSQLSessionManager(hiveServer: HiveServer2, sqlContext: SQLContext)
extends SessionManager(hiveServer)
with ReflectedCompositeService {
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason this PR removes ReflectedCompositeService?

setSuperField(this, "operationManager", sparkSqlOperationManager)
addService(sparkSqlOperationManager)

initCompositeService(hiveConf)
Copy link
Member

Choose a reason for hiding this comment

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

Why removing this line?

setSuperField(this, "operationManager", sparkSqlOperationManager)
addService(sparkSqlOperationManager)
Copy link
Member

Choose a reason for hiding this comment

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

This line is also needed, right?

@HyukjinKwon
Copy link
Member

Hi @yaooqinn, is this PR active? if so, would you address or answer to the review comments above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants