[SPARK-26992][STS] Fix STS scheduler pool correct delivery

[cxzl25]

What changes were proposed in this pull request?

The user sets the value of spark.sql.thriftserver.scheduler.pool.
Spark thrift server saves this value in the LocalProperty of threadlocal type, but does not clean up after running, causing other sessions to run in the previously set pool name.

How was this patch tested?

manual tests

[cxzl25]

The user sets the value of spark.sql.thriftserver.scheduler.pool.
Spark thrift server saves this value in the LocalProperty of threadlocal type, but does not clean up after running, causing other sessions to run in the previously set pool name.

For example
The second session does not manually set the pool name. The default pool name should be used, but the pool name of the previous user's settings is used. This is incorrect.

Fix

[cxzl25]

@srowen
I have already opened the RP.
Can get the current sql commit to which scheduling pool from spark ui.

[srowen]

Although this looks like it undoes the previous fix, you're saying it will always be null when this method starts right? It seems easier to unconditionally set it, unless there's some overhead I'm missing

[cxzl25]

Because only the user submitted sql has set spark.sql.thriftserver.scheduler.pool=xxx, sessionToActivePool will put xxx value, will not be the default scheduler, so the default scheduler pool name is null

Unconditional setting it is worried about ambiguity, because only after the put value, there will be a need to take this value, although there is no overhead

spark/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkExecuteStatementOperation.scala

Lines 236 to 239 in 43bf4ae

	case SetCommand(Some((SQLConf.THRIFTSERVER_POOL.key, Some(value)))) =>
	sessionToActivePool.put(parentSession.getSessionHandle, value)
	logInfo(s"Setting ${SparkContext.SPARK_SCHEDULER_POOL}=$value for future statements " +
	"in this session.")

[SparkQA]

Test build #4678 has finished for PR 23895 at commit 8ae9369.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

It seems reasonable; @caneGuy do you have thoughts on this? would it still address your issue?

[srowen]

Merged to master

[caneGuy]

@srowen sorry for replying so late. I think this solution is reasonable. Thanks @cxzl25