[SPARK-4103][SQL] Clean up SessionState in HiveContext by using ThreadLocal varaible in Hive #2967
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…HiveContext is lazy evaluated, and to start the session, SessionState.start() has to be invoked to initialize it. Using ThreadLocal variable natively in hive to track the availability of SessionState and initialize it if required. It avoid multiple thread sharing the same SessionState or single thread invoking SessionState.start with the same SessionState multiple times, which seems to cause unexpected behavior.
|
This PR may duplicate with Spark-4037 partially. @liancheng Please take a look at the code change |
|
Can one of the admins verify this patch? |
zhzhan
changed the title
| SessionState.start(ss) | ||
| ss.err = new PrintStream(outputBuffer, true, "UTF-8") | ||
| ss.out = new PrintStream(outputBuffer, true, "UTF-8") | ||
| } |
There was a problem hiding this comment.
If we use lazy eval, we have to relies on SessionState.start(sessionState) to initialize it. It may be invoked multiple times in various locations. Here we relies on checking ThreadLocal variable and initialize it at the first call to avoid this.
There was a problem hiding this comment.
Several comments here:
HiveContext.hiveconfshould be retrieved from the currentSessionStateifSessionState.get()is non-null. That's whyhiveconfandsessionStateare always initialized together in #2887- IO redirection and Hive properties propagation also need to be performed when
SessionState.get()returns a non-nullSessionStatestarted elsewhere. This is required by Spark SQL CLI sinceSparkSQLCLIDrivercreates aCliSessionStatebeforeHiveContextinitialization. - I guess you're trying to make
HiveContextplay well with multi-session scenarios by introducinggetSessionState()here? I had also considered this issue earlier, but unfortunately IMO proper multiple Hive session support may require major refactoring overHiveContext(e.g. make it absolutely thread safe). Also, Spark SQL Hive support will probably be reimplemented against the up coming foreign data source API. So I'd rather defer this major task later.
(As a globally used thread local object, Hive SessionState is really annoying...)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the sessionState in HiveContext is lazy evaluated, and to start the session, SessionState.start() has to be invoked to initialize it. Using ThreadLocal variable natively in hive to track the availability of SessionState and initialize it if required. It avoid multiple thread sharing the same SessionState or single thread invoke SessionState.start with the same SessionState multiple times, which seems to cause unexpected behavior, and break the contract defined in hive. Refer below comments form SessionState in hive.
/**
*/
public static SessionState start(SessionState startSs) {
...
}