You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/05/01 01:30:57 UTC
[GitHub] [spark] wangyum opened a new pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
wangyum opened a new pull request #32410:
URL: https://github.com/apache/spark/pull/32410
### What changes were proposed in this pull request?
This PR replaces `SessionState.start` with `SessionState.setCurrentSessionState`.
### Why are the changes needed?
To avoid [SessionState.createSessionDirs](https://github.com/apache/hive/blob/rel/release-2.3.8/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java#L652-L696) create too many directories:
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
// TODO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r648814524
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
We have disabled the session history:
https://github.com/apache/spark/blob/ebb4858f7185c6525adc4b23bc89f0a8262bf940/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala#L1271-L1276
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832562832
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138171/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832554678
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42692/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832408081
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42687/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832489273
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42690/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832561921
**[Test build #138171 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138171/testReport)** for PR 32410 at commit [`a6dfd95`](https://github.com/apache/spark/commit/a6dfd95c649113ba97fa3a3422c550ed29e6fdd7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830491879
**[Test build #138120 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138120/testReport)** for PR 32410 at commit [`96bbaeb`](https://github.com/apache/spark/commit/96bbaeb2cff6d7b456356382589c0f8865e034f6).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830635505
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42650/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830626967
**[Test build #138129 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138129/testReport)** for PR 32410 at commit [`2d046e2`](https://github.com/apache/spark/commit/2d046e20f2f9c46fc75766df116abddad140f069).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626216922
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,11 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ if (version != hive.v12) {
+ SessionState.setCurrentSessionState(state)
Review comment:
Please see [SessionState.start](https://github.com/apache/hive/blob/4ef83ee08b45c1ff7c6e894c45a0cf0acbf59b8a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java#L670-L751). It mainly does three things:
1. setCurrentSessionState
2. Initialize hiveHist
3. createSessionDirs
Spark SQL only needs setCurrentSessionState.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832499825
**[Test build #138171 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138171/testReport)** for PR 32410 at commit [`a6dfd95`](https://github.com/apache/spark/commit/a6dfd95c649113ba97fa3a3422c550ed29e6fdd7).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832378436
**[Test build #138166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138166/testReport)** for PR 32410 at commit [`a8306cb`](https://github.com/apache/spark/commit/a8306cb38b4a1d05dd724a855f53d1861d63bc3d).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-841799326
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-831480082
Got it. Thank you for the clarification, @wangyum .
> This patch use setCurrentSessionState with all versions except Hive 0.12.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840343604
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43016/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840378127
**[Test build #138496 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138496/testReport)** for PR 32410 at commit [`4bca8ec`](https://github.com/apache/spark/commit/4bca8ecaec066ef19d04a12e134ba830320a2e0f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832562150
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42692/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626214889
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,11 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ if (version != hive.v12) {
+ SessionState.setCurrentSessionState(state)
Review comment:
Used by Hive queries for testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832458662
**[Test build #138169 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138169/testReport)** for PR 32410 at commit [`3152a87`](https://github.com/apache/spark/commit/3152a876e75a1e2bc909ec8563fb30fc28f86d9c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832499825
**[Test build #138171 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138171/testReport)** for PR 32410 at commit [`a6dfd95`](https://github.com/apache/spark/commit/a6dfd95c649113ba97fa3a3422c550ed29e6fdd7).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626214739
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/CLIService.java
##########
@@ -126,7 +126,7 @@ private void applyAuthorizationConfigPolicy(HiveConf newHiveConf) throws HiveExc
// authorization and authentication are not session specific settings
SessionState ss = new SessionState(newHiveConf);
ss.setIsHiveServerQuery(true);
- SessionState.start(ss);
Review comment:
Revert this change because Spark use `SparkSQLCLIService`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830485717
**[Test build #138120 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138120/testReport)** for PR 32410 at commit [`96bbaeb`](https://github.com/apache/spark/commit/96bbaeb2cff6d7b456356382589c0f8865e034f6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r625335398
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,7 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
hmm just curious why we don't need to start the new session in these places. Maybe add more details in the PR description?
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,11 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ if (version != hive.v12) {
+ SessionState.setCurrentSessionState(state)
Review comment:
How useful is this? why don't we start the session in `newState`? BTW there is already `HiveShim.setCurrentSessionState`.
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/CLIService.java
##########
@@ -126,7 +126,7 @@ private void applyAuthorizationConfigPolicy(HiveConf newHiveConf) throws HiveExc
// authorization and authentication are not session specific settings
SessionState ss = new SessionState(newHiveConf);
ss.setIsHiveServerQuery(true);
- SessionState.start(ss);
Review comment:
Seems there is a bug originally here as the session state is not closed and therefore will leave the session directories orphan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum edited a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum edited a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830716144
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840310594
**[Test build #138496 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138496/testReport)** for PR 32410 at commit [`4bca8ec`](https://github.com/apache/spark/commit/4bca8ecaec066ef19d04a12e134ba830320a2e0f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830597347
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42647/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-831485837
cc @sunchao , too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832413283
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138166/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830495178
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832409563
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42687/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626215549
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,6 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
Review comment:
Spark SQL does not need SessionDirs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840310594
**[Test build #138496 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138496/testReport)** for PR 32410 at commit [`4bca8ec`](https://github.com/apache/spark/commit/4bca8ecaec066ef19d04a12e134ba830320a2e0f).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #32410:
URL: https://github.com/apache/spark/pull/32410
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r648265495
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,7 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ shim.setCurrentSessionState(state)
Review comment:
Sorry for late replay. It is two questions.
> if this is a safe change
Yes. The difference is `sessionState.start` creating too many directories and Spark SQL do not need it.
> why do you think this is making it simpler?
`shim.setCurrentSessionState(state)` is simpler than `if (version != hive.v12) SessionState.setCurrentSessionState(state) else SessionState.start(state) `.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r627099780
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,7 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Is it safe when we use `ADD JARS`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830492025
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42641/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832489273
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42690/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-841799326
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830582502
**[Test build #138126 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138126/testReport)** for PR 32410 at commit [`a75c625`](https://github.com/apache/spark/commit/a75c6259f05a480323f7f39b10de7e35ebac1146).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830635505
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42650/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bogdanghit commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
bogdanghit commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r650784376
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -828,6 +828,8 @@ private[hive] class HiveClientImpl(
}
}
+ // Hive query needs to start SessionState.
+ SessionState.start(state)
Review comment:
@HyukjinKwon @wangyum should this also be replaced with `shim.setCurrentSessionState(state)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832557356
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42692/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832492165
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138169/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830642006
**[Test build #138129 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138129/testReport)** for PR 32410 at commit [`2d046e2`](https://github.com/apache/spark/commit/2d046e20f2f9c46fc75766df116abddad140f069).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840378916
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138496/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r648786800
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Hm, actually @wangyum. I just noticed that this is different from Hive's https://github.com/apache/hive/blob/master/service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java#L180.
Shouldn't we at least keep:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java#L692-L699
for session history?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r625313108
##########
File path: sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/ThriftServerWithSparkContextSuite.scala
##########
@@ -28,8 +28,8 @@ 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") {
- assert(tempScratchDir.exists())
+ test("the scratch dir will not be exist") {
Review comment:
`be exist` -> `be created`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r650064530
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
It has removed by https://github.com/apache/hive/commit/9bebaf6196c1842c285bfbad50765170e064f6e4.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830716144
@dongjoon-hyun This patch only use setCurrentSessionState with all versions except for Hive 0.12.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832409579
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42687/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] bogdanghit commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
bogdanghit commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r646899104
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,7 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ shim.setCurrentSessionState(state)
Review comment:
@wangyum I wonder if this is a safe change. And why do you think this is making it simpler?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830485717
**[Test build #138120 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138120/testReport)** for PR 32410 at commit [`96bbaeb`](https://github.com/apache/spark/commit/96bbaeb2cff6d7b456356382589c0f8865e034f6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830597594
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138126/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r649093532
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Thanks Yuming. What about other codes? I know we don't have to take care about Tez but wonder if it's safe for other codes there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r629020979
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,7 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Yes. It is safe when use `ADD JARS`. We have disabled creating these directories for more than a year with the following changes(`HiveConf.ConfVars.WITHSCRATCHDIR=false`):
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830582502
**[Test build #138126 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138126/testReport)** for PR 32410 at commit [`a75c625`](https://github.com/apache/spark/commit/a75c6259f05a480323f7f39b10de7e35ebac1146).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832413283
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138166/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830495177
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830632435
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42650/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832409579
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42687/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r649093532
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Thanks Yumming. What about other codes? I know we don't have to take care about Tez but wonder if it's safe for other codes there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830597594
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138126/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r649958997
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
@wangyum, last question. What about this? Is it safe to remove?
```
// Hive object instance should be created with a copy of the conf object. If the conf is
// shared with SessionState, other parts of the code might update the config, but
// Hive.get(HiveConf) would not recognize the case when it needs refreshing
Hive.get(new HiveConf(startSs.conf)).getMSC();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830597514
**[Test build #138126 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138126/testReport)** for PR 32410 at commit [`a75c625`](https://github.com/apache/spark/commit/a75c6259f05a480323f7f39b10de7e35ebac1146).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830626967
**[Test build #138129 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138129/testReport)** for PR 32410 at commit [`2d046e2`](https://github.com/apache/spark/commit/2d046e20f2f9c46fc75766df116abddad140f069).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626215655
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,7 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Spark SQL does not need SessionDirs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830597347
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42647/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r649112996
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
Other codes also do not needed. We have disabled creating these directories for more than a year with the following changes:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r627092169
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,11 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ if (version != hive.v12) {
+ SessionState.setCurrentSessionState(state)
Review comment:
Make sense. I was curious why we call `setCurrentSessionState` here but then call `start` later in `runHive`. It seems that is only used for testing so all good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840343550
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r626215549
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,6 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
Review comment:
Spark SQL does not need SessionDirs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r650794650
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -828,6 +828,8 @@ private[hive] class HiveClientImpl(
}
}
+ // Hive query needs to start SessionState.
+ SessionState.start(state)
Review comment:
I think it's because it executes some Hive queries e.g.) `sparkSession.metadataHive.runSqlHive("SELECT * FROM t")` at `sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveMetastoreCatalogSuite.scala`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r627101614
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,7 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
And, I'm wondering about what happens to `LOCALSCRATCHDIR` after this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [WIP][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830594432
Kubernetes integration test unable to build dist.
exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42647/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832492165
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138169/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832562832
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138171/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832483363
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42690/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832458662
**[Test build #138169 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138169/testReport)** for PR 32410 at commit [`3152a87`](https://github.com/apache/spark/commit/3152a876e75a1e2bc909ec8563fb30fc28f86d9c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r650828093
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -828,6 +828,8 @@ private[hive] class HiveClientImpl(
}
}
+ // Hive query needs to start SessionState.
+ SessionState.start(state)
Review comment:
Yes. Hive query needs to start `SessionState`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830643637
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138129/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r632306070
##########
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##########
@@ -190,7 +190,7 @@ private[hive] class HiveClientImpl(
// For this reason we cannot load the jars added by ADDJarsCommand because of class loader
// got changed. We reset it to clientLoader.ClassLoader here.
state.getConf.setClassLoader(clientLoader.classLoader)
- SessionState.start(state)
+ shim.setCurrentSessionState(state)
Review comment:
@HyukjinKwon @sunchao I changed it to `shim.setCurrentSessionState(state)` to make it simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832486614
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42690/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840343604
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43016/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832491797
**[Test build #138169 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138169/testReport)** for PR 32410 at commit [`3152a87`](https://github.com/apache/spark/commit/3152a876e75a1e2bc909ec8563fb30fc28f86d9c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait ExtractValue extends Expression `
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-830643637
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138129/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #32410:
URL: https://github.com/apache/spark/pull/32410
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32410:
URL: https://github.com/apache/spark/pull/32410#discussion_r648787117
##########
File path: sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
##########
@@ -141,7 +141,8 @@ public void open(Map<String, String> sessionConfMap) throws HiveSQLException {
sessionState = new SessionState(hiveConf, username);
sessionState.setUserIpAddress(ipAddress);
sessionState.setIsHiveServerQuery(true);
- SessionState.start(sessionState);
+ // Use setCurrentSessionState to avoid creating useless SessionDirs.
+ SessionState.setCurrentSessionState(sessionState);
Review comment:
It would be great if we can justify line by line. I think we won't have to take care about Tez but what about others here:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java#L688-L766
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832412421
**[Test build #138166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138166/testReport)** for PR 32410 at commit [`a8306cb`](https://github.com/apache/spark/commit/a8306cb38b4a1d05dd724a855f53d1861d63bc3d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832378436
**[Test build #138166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138166/testReport)** for PR 32410 at commit [`a8306cb`](https://github.com/apache/spark/commit/a8306cb38b4a1d05dd724a855f53d1861d63bc3d).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-832562150
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/42692/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32410: [SPARK-35286][SQL] Replace SessionState.start with SessionState.setCurrentSessionState
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32410:
URL: https://github.com/apache/spark/pull/32410#issuecomment-840378916
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138496/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org