-
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-13648] Add Hive Cli to classes for isolated classloader #11495
Conversation
Copy of recent development mailing list comment. I have been testing 1.6.1RC1 using the IBM Java SDK. I notice a problem ( with the org.apache.spark.sql.hive.client.VersionsSuite tests ) after a recent Spark 1.6.1 change. The change introduced a dependency on org.apache.hadoop.hive.cli.CliSessionState in org.apache.spark.sql.hive.client.ClientWrapper. In particular the following test was added The problem is that VersionsSuite test uses an isolated classloader in order to test various versions of Hive. However the classpath of the isolated classloader does not contain CliSessionState. The behaviour of isInstanceOf[CliSessionState]) is JVM vendor specific. In particular whether this code causes the CliSessionState class to be loaded. ( It does not for openjdk, but does for IBM JDK ). Hence this call can throw a classnotfound exception. |
cc @marmbrus |
ok to test |
LGTM pending tests |
Test build #52418 has finished for PR 11495 at commit
|
Thanks, merging to master and 1.6. |
## What changes were proposed in this pull request? Adding the hive-cli classes to the classloader ## How was this patch tested? The hive Versionssuite tests were run This is my original work and I license the work to the project under the project's open source license. Author: Tim Preece <[email protected]> Closes #11495 from preecet/master. (cherry picked from commit 46f25c2) Signed-off-by: Michael Armbrust <[email protected]>
Why we need |
Because ClientWrapper calls isInstanceOf[CliSessionState] and so needs to load the class CliSessionstate. In the testcase ClientWraper has been loaded by the isolated class loader. |
## What changes were proposed in this pull request? Adding the hive-cli classes to the classloader ## How was this patch tested? The hive Versionssuite tests were run This is my original work and I license the work to the project under the project's open source license. Author: Tim Preece <[email protected]> Closes apache#11495 from preecet/master.
The `hive` subproject currently depends on `hive-cli` in order to perform a check to see whether a `SessionState` is an instance of `org.apache.hadoop.hive.cli.CliSessionState` (see #9589). The introduction of this `hive-cli` dependency has caused problems for users whose Hive metastore JAR classpaths don't include the `hive-cli` classes (such as in #11495). This patch removes this dependency on `hive-cli` and replaces the `isInstanceOf` check by reflection. I added a Maven Enforcer rule to ban `hive-cli` from the `hive` subproject in order to make sure that this dependency is not accidentally reintroduced. /cc rxin yhuai adrian-wang preecet Author: Josh Rosen <[email protected]> Closes #12551 from JoshRosen/remove-hive-cli-dep-from-hive-subproject.
What changes were proposed in this pull request?
Adding the hive-cli classes to the classloader
How was this patch tested?
The hive Versionssuite tests were run
This is my original work and I license the work to the project under the project's open source license.