Skip to content

Conversation

@yhuai
Copy link
Contributor

@yhuai yhuai commented Jun 11, 2015

@yhuai
Copy link
Contributor Author

yhuai commented Jun 11, 2015

hmm..Actually, setCurrentSessionState will use the class loader associated with the HiveConf set in the current session state to the context class loader of the current thread.

@yhuai
Copy link
Contributor Author

yhuai commented Jun 11, 2015

Actually, the problem is the class loader associated with the HiveConf inside the ClientWrapper.state. For execution Hive, this classloader will be the one used to load HiveContext. Then, when we call withHiveState, we always call setCurrentSessionState with this state. Inside setCurrentSessionState, we are doing

public static void setCurrentSessionState(SessionState startSs) {
    tss.set(startSs);
    Thread.currentThread().setContextClassLoader(startSs.getConf().getClassLoader());
  }

Basically, we use the classloader of the conf associated with the state to set the context class loader of the current thread.

Inside AddJar command, although we are doing org.apache.hadoop.hive.ql.metadata.Hive.get().getConf().setClassLoader(newClassLoader), this one actually may not set the class loader to the conf associated with executionHive.state because org.apache.hadoop.hive.ql.metadata.Hive.get() returns a thread local variable. So, its conf may not point to the same conf inside the executionHive.state.

Also cc @JoshRosen

@yhuai yhuai changed the title [SPARK-8306] executionHive does not need to use switch class loader [SPARK-8306] AddJar command needs to set the new class loader to the HiveConf inside executionHive.state. Jun 11, 2015
@yhuai
Copy link
Contributor Author

yhuai commented Jun 11, 2015

Manually tested. Looks fine. Will try to add a test.

@SparkQA
Copy link

SparkQA commented Jun 11, 2015

Test build #34669 timed out for PR 6758 at commit 5c8b794 after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Jun 11, 2015

Test build #34679 has finished for PR 6758 at commit 3d541fd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is very likely to work for almost all cases, but we should probably add a TODO or JIRA to clean up some of this stuff. For instance, here we may be setting the executionHive class loader to an entirely different class loader than it originally had (which is likely to work in typical situations, but is not easy to trace).

Additionally, the flow of class loader-setting inside ClientWrapper is very non-obvious, especially as it differs between Hive 12 and Hive 13 -- we should explicitly document this if not make it cleaner.

@yhuai yhuai changed the title [SPARK-8306] AddJar command needs to set the new class loader to the HiveConf inside executionHive.state. [SPARK-8306] [SQL] AddJar command needs to set the new class loader to the HiveConf inside executionHive.state. Jun 17, 2015
@SparkQA
Copy link

SparkQA commented Jun 17, 2015

Test build #35055 has finished for PR 6758 at commit 6690a08.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 17, 2015

Test build #35057 has finished for PR 6758 at commit 1292346.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class UnaryPositive(child: Expression) extends UnaryArithmetic
    • // Add jar to isolated hive (metadataHive) class loader.

@asfgit asfgit closed this in 302556f Jun 17, 2015
asfgit pushed a commit that referenced this pull request Jun 17, 2015
…o the HiveConf inside executionHive.state.

https://issues.apache.org/jira/browse/SPARK-8306

I will try to add a test later.

marmbrus aarondav

Author: Yin Huai <[email protected]>

Closes #6758 from yhuai/SPARK-8306 and squashes the following commits:

1292346 [Yin Huai] [SPARK-8306] AddJar command needs to set the new class loader to the HiveConf inside executionHive.state.

(cherry picked from commit 302556f)
Signed-off-by: Michael Armbrust <[email protected]>

Conflicts:
	sql/hive/src/main/scala/org/apache/spark/sql/hive/client/ClientWrapper.scala
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…o the HiveConf inside executionHive.state.

https://issues.apache.org/jira/browse/SPARK-8306

I will try to add a test later.

marmbrus aarondav

Author: Yin Huai <[email protected]>

Closes apache#6758 from yhuai/SPARK-8306 and squashes the following commits:

1292346 [Yin Huai] [SPARK-8306] AddJar command needs to set the new class loader to the HiveConf inside executionHive.state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants