Skip to content

[SPARK-19084][sql] Ensure context class loader is set when initializing Hive. #17154

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.

Sign up for GitHub

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

Closed
vanzin wants to merge 2 commits into from
Closed

[SPARK-19084][sql] Ensure context class loader is set when initializing Hive. #17154

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3246196
[SPARK-19084][sql] Ensure context class loader is set when initializi…
Mar 3, 2017
aa2a66b
Add comment.
Mar 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,16 +268,19 @@ private[hive] class HiveClientImpl(
*/
def withHiveState[A](f: => A): A = retryLocked {
val original = Thread.currentThread().getContextClassLoader
// Set the thread local metastore client to the client associated with this HiveClientImpl.
Hive.set(client)
val originalConfLoader = state.getConf.getClassLoader
// The classloader in clientLoader could be changed after addJar, always use the latest
// classloader
Thread.currentThread().setContextClassLoader(clientLoader.classLoader)
Copy link
Contributor

@cloud-fan cloud-fan Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we add some comments to say why we need to do this?

Copy link
Contributor Author

@vanzin vanzin Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

state.getConf.setClassLoader(clientLoader.classLoader)
Copy link
Contributor

@cloud-fan cloud-fan Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like we fixed 2 problems here: 1. set thread context class loader. 2. set class loader back for state.getConf

Copy link
Contributor Author

@vanzin vanzin Mar 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; I'm not sure the second one is strictly necessary, but from a cleanliness p.o.v. it looks like the right thing to do.

// Set the thread local metastore client to the client associated with this HiveClientImpl.
Hive.set(client)
// setCurrentSessionState will use the classLoader associated
// with the HiveConf in `state` to override the context class loader of the current
// thread.
shim.setCurrentSessionState(state)
val ret = try f finally {
state.getConf.setClassLoader(originalConfLoader)
Thread.currentThread().setContextClassLoader(original)
HiveCatalogMetrics.incrementHiveClientCalls(1)
}
Expand Down