SPARK-3025 [SQL]: Allow JDBC clients to set a fair scheduler pool

[pwendell]

This definitely needs review as I am not familiar with this part of Spark.
I tested this locally and it did seem to work.

[SparkQA]

QA tests have started for PR 1937. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18517/consoleFull

[SparkQA]

QA results for PR 1937:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18517/consoleFull

[liancheng]

retest this please

[SparkQA]

QA tests have started for PR 1937. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18541/consoleFull

[SparkQA]

QA results for PR 1937:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18541/consoleFull

[tianyi]

hi @pwendell , I have two questions:
1 Why don't handle this set command in "SetCommand" ?
2 Do we have to save this pool name in LocalProperty?

[pwendell]

@tianyi it might make more sense to handle this in the physical execution of SetCommand, I'll see if I can do that. The local property is necessary I think - do you know of an alternative mechanism? IIRC this is the main way we expect people to allocate jobs to specific pools.

[pwendell]

@tianyi after looking some more, I believe this needs to be handled in the thriftserver explicitly and cannot be handled in the SetCommand physical operator. The reason is that we need to track association with the specific Hive session. To make this more clear, I'm going to rename this setting to include the word "thriftserver" so it's clear that this is only relevant to the thriftserver. In the case where a user is referencing a SQLContext directly, they can simply set the local property themeselves.

[pwendell]

Jenkins, test this please.

[SparkQA]

QA tests have started for PR 1937 at commit 2c647e0.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 1937 at commit 2c647e0.

• This patch fails unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

QA tests have started for PR 1937 at commit b858e33.

• This patch merges cleanly.

[SparkQA]

QA tests have finished for PR 1937 at commit b858e33.

• This patch fails unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[tianyi]

@pwendell thanks for the explanation. I think there should be some test case about this feature.

[tianyi]

@pwendell I think we could save the current session's pool setting in the hiveConf of parentSession instead of a map in SparkSQLOperationManager. So it could resolve the unlimit map size problem.

[marmbrus]

As far as I know we don't actually do much with the HiveConf that is stored in parentSession, though that does seem like a reasonable place to store such information. There is already a JIRA open for cleaning up multi-user semantics in the JDBC server. Given that its be nice to have something here for 1.1 I'm going to go ahead and merge this, and leave a note in that JIRA to clean this up when we have a better solution for session state.