Skip to content

[SPARK-33376][SQL] Remove the option of "sharesHadoopClasses" in Hive IsolatedClientLoader #30284

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
sunchao wants to merge 2 commits into from
Closed

[SPARK-33376][SQL] Remove the option of "sharesHadoopClasses" in Hive IsolatedClientLoader #30284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b735947
Remove shares Hadoop classes
sunchao Nov 5, 2020
0b035b2
Address comments
sunchao Nov 10, 2020
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
13 changes: 3 additions & 10 deletions sql/hive/src/main/scala/org/apache/spark/sql/hive/client/IsolatedClientLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,9 @@ private[hive] object IsolatedClientLoader extends Logging {
config: Map[String, String] = Map.empty,
ivyPath: Option[String] = None,
sharedPrefixes: Seq[String] = Seq.empty,
barrierPrefixes: Seq[String] = Seq.empty,
sharesHadoopClasses: Boolean = true): IsolatedClientLoader = synchronized {
barrierPrefixes: Seq[String] = Seq.empty): IsolatedClientLoader = synchronized {
val resolvedVersion = hiveVersion(hiveMetastoreVersion)
// We will first try to share Hadoop classes. If we cannot resolve the Hadoop artifact
// with the given version, we will use Hadoop 2.7 and then will not share Hadoop classes.
var _sharesHadoopClasses = sharesHadoopClasses
// We will use Hadoop 2.7 if we cannot resolve the Hadoop artifact.
val files = if (resolvedVersions.contains((resolvedVersion, hadoopVersion))) {
resolvedVersions((resolvedVersion, hadoopVersion))
} else {
Expand All @@ -75,7 +72,6 @@ private[hive] object IsolatedClientLoader extends Logging {
"again. Hadoop classes will not be shared between Spark and Hive metastore client. " +

@HyukjinKwon HyukjinKwon Nov 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shall we update this message since we now always share? Actually we should think about throwing an exception that says all related metastore jars have to be specified instead.

@sunchao sunchao Nov 9, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Forgot about this place - will fix. Yes agree that an exception might be more appropriate. It seems a bit arbitrary to fall back to 2.7.4 here since the original Hadoop version could vary based on the Hadoop profile (either 2.7.4 or 3.2.0 at the moment).

"It is recommended to set jars used by Hive metastore client through " +
"spark.sql.hive.metastore.jars in the production environment.")
_sharesHadoopClasses = false
(downloadVersion(
resolvedVersion, fallbackVersion, ivyPath, remoteRepos), fallbackVersion)
}
Expand All @@ -89,7 +85,6 @@ private[hive] object IsolatedClientLoader extends Logging {
execJars = files,
hadoopConf = hadoopConf,
config = config,
sharesHadoopClasses = _sharesHadoopClasses,
sharedPrefixes = sharedPrefixes,
barrierPrefixes = barrierPrefixes)
}
Expand Down Expand Up @@ -177,7 +172,6 @@ private[hive] object IsolatedClientLoader extends Logging {
* @param config A set of options that will be added to the HiveConf of the constructed client.
* @param isolationOn When true, custom versions of barrier classes will be constructed. Must be
* true unless loading the version of hive that is on Spark's classloader.
* @param sharesHadoopClasses When true, we will share Hadoop classes between Spark and
* @param baseClassLoader The spark classloader that is used to load shared classes.
*/
private[hive] class IsolatedClientLoader(
Expand All @@ -187,7 +181,6 @@ private[hive] class IsolatedClientLoader(
val execJars: Seq[URL] = Seq.empty,
val config: Map[String, String] = Map.empty,
val isolationOn: Boolean = true,
val sharesHadoopClasses: Boolean = true,
val baseClassLoader: ClassLoader = Thread.currentThread().getContextClassLoader,
val sharedPrefixes: Seq[String] = Seq.empty,
val barrierPrefixes: Seq[String] = Seq.empty)
Expand All @@ -204,7 +197,7 @@ private[hive] class IsolatedClientLoader(
name.startsWith("org.apache.log4j") || // log4j1.x
name.startsWith("org.apache.logging.log4j") || // log4j2
name.startsWith("org.apache.spark.") ||
(sharesHadoopClasses && isHadoopClass) ||
isHadoopClass ||
name.startsWith("scala.") ||
(name.startsWith("com.google") && !name.startsWith("com.google.cloud")) ||
name.startsWith("java.") ||
Expand Down
3 changes: 1 addition & 2 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HadoopVersionInfoSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ class HadoopVersionInfoSuite extends SparkFunSuite {
sparkConf = new SparkConf(),
hadoopConf = hadoopConf,
config = HiveClientBuilder.buildConf(Map.empty),
ivyPath = Some(ivyPath.getCanonicalPath),
sharesHadoopClasses = true)
ivyPath = Some(ivyPath.getCanonicalPath))
val jars = client.classLoader.getParent.asInstanceOf[URLClassLoader].getURLs
.map(u => new File(u.toURI))
// Drop all Hadoop jars to use the existing Hadoop jars on the classpath
Expand Down
6 changes: 2 additions & 4 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveClientBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,13 @@ private[client] object HiveClientBuilder {
def buildClient(
version: String,
hadoopConf: Configuration,
extraConf: Map[String, String] = Map.empty,
sharesHadoopClasses: Boolean = true): HiveClient = {
extraConf: Map[String, String] = Map.empty): HiveClient = {
IsolatedClientLoader.forVersion(
hiveMetastoreVersion = version,
hadoopVersion = VersionInfo.getVersion,
sparkConf = new SparkConf(),
hadoopConf = hadoopConf,
config = buildConf(extraConf),
ivyPath = ivyPath,
sharesHadoopClasses = sharesHadoopClasses).createClient()
ivyPath = ivyPath).createClient()
}
}
4 changes: 0 additions & 4 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HivePartitionFilteringSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,6 @@ class HivePartitionFilteringSuite(version: String)
day1 :: day2 :: Nil)
}

test("create client with sharesHadoopClasses = false") {
buildClient(new Configuration(), sharesHadoopClasses = false)
}

private def testMetastorePartitionFiltering(
filterExpr: Expression,
expectedDs: Seq[Int],
Expand Down
7 changes: 2 additions & 5 deletions sql/hive/src/test/scala/org/apache/spark/sql/hive/client/HiveVersionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ private[client] abstract class HiveVersionSuite(version: String) extends SparkFu
override protected val enableAutoThreadAudit = false
protected var client: HiveClient = null

protected def buildClient(
hadoopConf: Configuration,
sharesHadoopClasses: Boolean = true): HiveClient = {
protected def buildClient(hadoopConf: Configuration): HiveClient = {
// Hive changed the default of datanucleus.schema.autoCreateAll from true to false and
// hive.metastore.schema.verification from false to true since 2.0
// For details, see the JIRA HIVE-6113 and HIVE-12463
Expand All @@ -46,8 +44,7 @@ private[client] abstract class HiveVersionSuite(version: String) extends SparkFu
HiveClientBuilder.buildClient(
version,
hadoopConf,
HiveUtils.formatTimeVarsForHiveClient(hadoopConf),
sharesHadoopClasses = sharesHadoopClasses)
HiveUtils.formatTimeVarsForHiveClient(hadoopConf))
}

override def suiteName: String = s"${super.suiteName}($version)"
Expand Down