-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-21771][SQL]remove useless hive client in SparkSQLEnv #18983
Conversation
Once a meta hive client is created, it generates its SessionState which creates a lot of session related directories, some deleteOnExit, some does not. if a hive client is useless we may not create it at the very start.
cc @hvanhovell |
@hvanhovell Is there any specific reason that we still need to create a new |
ok to test |
Test build #86361 has finished for PR 18983 at commit
|
Seems it should be okay to remove all the |
besides, redirecting output here only is needed when there is an instance of a CliSessionState, otherwise, it will be done during the SessionState initializing in HiveClientImpl |
LGTM, although I'm not very familiar with the thrift server code... |
cc @liufengdb |
LGTM! It is only created once though. Frankly, we should completely remove the implementation of |
Thanks! Merged to master/2.3 |
## What changes were proposed in this pull request? Once a meta hive client is created, it generates its SessionState which creates a lot of session related directories, some deleteOnExit, some does not. if a hive client is useless we may not create it at the very start. ## How was this patch tested? N/A cc hvanhovell cloud-fan Author: Kent Yao <11215016@zju.edu.cn> Closes #18983 from yaooqinn/patch-1. (cherry picked from commit 793841c) Signed-off-by: gatorsmile <gatorsmile@gmail.com>
What changes were proposed in this pull request?
Once a meta hive client is created, it generates its SessionState which creates a lot of session related directories, some deleteOnExit, some does not. if a hive client is useless we may not create it at the very start.
How was this patch tested?
N/A
cc @hvanhovell @cloud-fan