[SPARK-32852][SQL] spark.sql.hive.metastore.jars support HDFS location #29881

AngersZhuuuu · 2020-09-27T04:20:05Z

What changes were proposed in this pull request?

Support spark.sql.hive.metastore.jars use HDFS location.

When user need to use path to set hive metastore jars, you should set
spark.sql.hive.metasstore.jars=path and set real path in spark.sql.hive.metastore.jars.path
since we use File.pathSeperator to split path, but FIle.pathSeparator is : in unix, it will split hdfs location hdfs://nameservice/xx. So add new config spark.sql.hive.metastore.jars.path to set comma separated paths.
To keep both two way supported

Why are the changes needed?

All spark app can fetch internal version hive jars in HDFS location, not need distribute to all node.

Does this PR introduce any user-facing change?

User can use HDFS location to store hive metastore jars

How was this patch tested?

Manuel tested.

AngersZhuuuu · 2020-09-27T04:20:14Z

ping @wangyum

SparkQA · 2020-09-27T04:28:34Z

Test build #129140 has finished for PR 29881 at commit 834c2a7.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-27T05:11:04Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33756/

SparkQA · 2020-09-27T05:24:00Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33757/

SparkQA · 2020-09-27T05:29:08Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33756/

SparkQA · 2020-09-27T05:40:29Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33757/

SparkQA · 2020-09-27T06:37:45Z

Test build #129141 has finished for PR 29881 at commit bf3c4b6.

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

dongjoon-hyun

Hi, @AngersZhuuuu . Is this PR limited to HDFS? What about S3 storage?

AngersZhuuuu · 2020-09-29T06:06:41Z

Hi, @AngersZhuuuu . Is this PR limited to HDFS? What about S3 storage?

I will test for s3.

SparkQA · 2020-09-29T09:25:42Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33839/

SparkQA · 2020-09-29T09:27:47Z

Test build #129224 has finished for PR 29881 at commit 1063a08.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-09-29T09:29:43Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33840/

SparkQA · 2020-09-29T09:42:56Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33839/

SparkQA · 2020-09-29T09:48:01Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33840/

SparkQA · 2020-09-29T10:27:04Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33846/

SparkQA · 2020-09-29T10:43:33Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33846/

HyukjinKwon · 2020-09-29T11:34:50Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+      // Convert to files and expand any directories.
+      val jars =
+        hiveMetastoreJars
+          .split(";")


Why do we change this from File.pathSeparator to ;?

Why do we change this from File.pathSeparator to ;?

Since in unix system, File.pathSeparator is : will split hdfs path hdfs://xxx/xxx

Path separator is OS dependent.. lets don't change the behaviour

Path separator is OS dependent.. lets don't change the behaviour

But a lot place we use comma separator to split file path, right? Also I am confused that why we must need to use this Path separator here since what we config is jar file list or *.

or you mean use will pass config like hadoop classpath, but seem no hive classpath

ping @dongjoon-hyun @wangyum @cloud-fan WDYT about this problem

Hey, yes using comma might be a good way but now you're breaking the user applications that use :. Let's don't break.
I think path separator is taken after the regular classpath behaviour when you run jars like java -cp.

HyukjinKwon · 2020-09-29T11:37:29Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+        hiveMetastoreJars
+          .split(";")
+          .flatMap {
+            case path if path.contains("\\") =>


I could not follow this. Do you want to check if this is a Windows path? If that's the case, you can use Utils.isWindows.

\\ can be in file names in other OSs. I don't think it makes sense to assume that this is a local path.

\\ can be in file names in other OSs. I don't think it makes sense to assume that this is a local path.

All right, I will raise a pr to change this in SparkContext's addJar/addFile

I could not follow this. Do you want to check if this is a Windows path? If that's the case, you can use Utils.isWindows.

Here need use condition Utils.isWindows && path.contains("\\") since path maybe remote path

SparkQA · 2020-09-29T11:39:29Z

Test build #129230 has finished for PR 29881 at commit 6214ebf.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-09-29T11:42:12Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+              val uri = new Path(path).toUri
+              uri.getScheme match {
+                case null | "file" =>
+                  addLocalHiveJars(new File(uri.getPath))


Assuming null as a local path is kind of ambiguous (local paths vs fs.default.name). Maybe you'll have to document that it should always be fully qualified URL to indicate other file systems.

Assuming null as a local path is kind of ambiguous (local paths vs fs.default.name). Maybe you'll have to document that it should always be fully qualified URL to indicate other file systems.

Add comment in doc of conf spark.sql.hive.metastore.jars

Should this comment on the option "path" instead of "classpath"?

Should this comment on the option "path" instead of "classpath"?

Can't got your point, can you make it more clear?

HyukjinKwon · 2020-09-29T11:43:14Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+                case null | "file" =>
+                  addLocalHiveJars(new File(uri.getPath))
+                case "local" =>
+                  new File("file:" + uri.getPath).toURL :: Nil


Here I think you can treat local as same as file since we're not downloading or fetching it (or distribute)

Here I think you can treat local as same as file since we're not downloading or fetching it (or distribute)

Yae

cloud-fan · 2020-10-16T11:20:03Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+      s" 2. hdfs://nameservice/path/to/jar/xxx.jar" +
+      s" 3. /path/to/jar/ (path without URI scheme follow conf `fs.defaultFS`'s URI schema)" +
+      s" 4. [http/https/ftp]://path/to/jar/xxx.jar" +
+      s"For URI, we can't support path wildcard, but for other URL support nested path wildcard," +


can we be more specific? e.g. http/https/ftp doesn't support wildcard.

can we be more specific? e.g. http/https/ftp doesn't support wildcard.

Done

cloud-fan · 2020-10-16T11:20:38Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

      |   Use Hive jars of specified version downloaded from Maven repositories.
-      | 3. A classpath in the standard format for both Hive and Hadoop.
+      | 3. "path"
+      |   Use Hive jars configured by `spark.sql.hive.metastore.jars.path` in comma separated format


Add a period at the end?

Add a period at the end?

Done

SparkQA · 2020-10-17T03:36:20Z

Test build #129933 has finished for PR 29881 at commit d631a75.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-10-17T04:22:10Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34538/

SparkQA · 2020-10-17T04:44:35Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34538/

SparkQA · 2020-10-17T05:07:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34541/

SparkQA · 2020-10-17T05:38:26Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34541/

SparkQA · 2020-10-17T06:33:27Z

Test build #129936 has finished for PR 29881 at commit 6e67c7b.

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

AngersZhuuuu · 2020-10-19T11:09:59Z

ping @cloud-fan

cloud-fan · 2020-10-22T07:11:58Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+      | 3. "path"
+      |   Use Hive jars configured by `spark.sql.hive.metastore.jars.path`
+      |   in comma separated format. Support both local or remote paths, it should always
+      |   be fully qualified URL to indicate other file systems.


it should always be fully qualified URL ... it's not true now.

it should always be fully qualified URL ... it's not true now.

Remove this now

cloud-fan · 2020-10-22T07:12:23Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+      s" 2. hdfs://nameservice/path/to/jar/xxx.jar" +
+      s" 3. /path/to/jar/ (path without URI scheme follow conf `fs.defaultFS`'s URI schema)" +
+      s" 4. [http/https/ftp]://path/to/jar/xxx.jar" +
+      s"Notice: `http/https/ftp` doesn't support wildcard, but for other URLs support" +


but for other ... -> but other ...

but for other ... -> but other ...

Done

cloud-fan · 2020-10-22T07:13:08Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+  val HIVE_METASTORE_JARS_PATH = buildStaticConf("spark.sql.hive.metastore.jars.path")
+    .doc(s"Comma separated fully qualified URL of Hive jars, support both local and remote paths," +
+      s"Such as: " +
+      s" 1. file://path/to/jar/xxx.jar" +


shall we add \n at the end for each item?

shall we add \n at the end for each item?

Done

cloud-fan · 2020-10-22T07:13:17Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala

+      s"Notice: `http/https/ftp` doesn't support wildcard, but for other URLs support" +
+      s" nested path wildcard, Such as: " +
+      s" 1. file://path/to/jar/*, file://path/to/jar/*/*" +
+      s" 2. hdfs://nameservice/path/to/jar/*, hdfs://nameservice/path/to/jar/*/*" +


ditto, \n

ditto, \n

Done

SparkQA · 2020-10-22T09:29:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34748/

SparkQA · 2020-10-22T09:55:30Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34748/

SparkQA · 2020-10-22T10:39:45Z

Test build #130141 has finished for PR 29881 at commit ea9ef2b.

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

cloud-fan · 2020-10-22T13:52:40Z

thanks, merging to master!

…ql.hive.metastore.jars ### What changes were proposed in this pull request? This is a follow-up for apache/spark#29881. It revises the documentation of the configuration `spark.sql.hive.metastore.jars`. ### Why are the changes needed? Fix grammatical error in the doc. Also, make it more clear that the configuration is effective only when `spark.sql.hive.metastore.jars` is set as `path` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Just doc changes. Closes #30407 from gengliangwang/reviseJarPathDoc. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>

…istence when config hive jars location ### What changes were proposed in this pull request? Add notice about keep hive version consistence when config hive jars location With PR #29881, if we don't keep hive version consistence. we will got below error. ``` Builtin jars can only be used when hive execution version == hive metastore version. Execution: 2.3.8 != Metastore: 1.2.1. Specify a valid path to the correct hive jars using spark.sql.hive.metastore.jars or change spark.sql.hive.metastore.version to 2.3.8. ``` ![image](https://user-images.githubusercontent.com/46485123/105795169-512d8380-5fc7-11eb-97c3-0259a0d2aa58.png) ### Why are the changes needed? Make config doc detail ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Not need Closes #31317 from AngersZhuuuu/SPARK-32852-followup. Authored-by: Angerszhuuuu <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

…istence when config hive jars location ### What changes were proposed in this pull request? Add notice about keep hive version consistence when config hive jars location With PR #29881, if we don't keep hive version consistence. we will got below error. ``` Builtin jars can only be used when hive execution version == hive metastore version. Execution: 2.3.8 != Metastore: 1.2.1. Specify a valid path to the correct hive jars using spark.sql.hive.metastore.jars or change spark.sql.hive.metastore.version to 2.3.8. ``` ![image](https://user-images.githubusercontent.com/46485123/105795169-512d8380-5fc7-11eb-97c3-0259a0d2aa58.png) ### Why are the changes needed? Make config doc detail ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Not need Closes #31317 from AngersZhuuuu/SPARK-32852-followup. Authored-by: Angerszhuuuu <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 7bd4165) Signed-off-by: HyukjinKwon <[email protected]>

…istence when config hive jars location ### What changes were proposed in this pull request? Add notice about keep hive version consistence when config hive jars location With PR apache#29881, if we don't keep hive version consistence. we will got below error. ``` Builtin jars can only be used when hive execution version == hive metastore version. Execution: 2.3.8 != Metastore: 1.2.1. Specify a valid path to the correct hive jars using spark.sql.hive.metastore.jars or change spark.sql.hive.metastore.version to 2.3.8. ``` ![image](https://user-images.githubusercontent.com/46485123/105795169-512d8380-5fc7-11eb-97c3-0259a0d2aa58.png) ### Why are the changes needed? Make config doc detail ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Not need Closes apache#31317 from AngersZhuuuu/SPARK-32852-followup. Authored-by: Angerszhuuuu <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

SPARK-32852 spark.sql.hive.metastore.jars support HDFS location

834c2a7

probot-autolabeler bot added the SQL label Sep 27, 2020

Update HiveUtils.scala

bf3c4b6

HyukjinKwon changed the title ~~[SPARK-32852][SQL} spark.sql.hive.metastore.jars support HDFS location~~ [SPARK-32852][SQL] spark.sql.hive.metastore.jars support HDFS location Sep 27, 2020

dongjoon-hyun reviewed Sep 29, 2020

View reviewed changes

AngersZhuuuu added 2 commits September 29, 2020 16:03

update

2cfd3e0

.

1063a08

AngersZhuuuu added 2 commits September 29, 2020 17:41

Merge branch 'master' into SPARK-32852

6eaafea

Update HadoopVersionInfoSuite.scala

6214ebf

HyukjinKwon reviewed Sep 29, 2020

View reviewed changes

follow comment

e82ed52

cloud-fan reviewed Oct 16, 2020

View reviewed changes

Update HiveUtils.scala

d631a75

AngersZhuuuu added 2 commits October 17, 2020 12:19

Update HiveUtils.scala

efc5ae0

Update HiveUtils.scala

6e67c7b

cloud-fan reviewed Oct 22, 2020

View reviewed changes

cloud-fan approved these changes Oct 22, 2020

View reviewed changes

Update HiveUtils.scala

ea9ef2b

cloud-fan closed this in a1629b4 Oct 22, 2020

gengliangwang mentioned this pull request Nov 18, 2020

[SPARK-32852][SQL][Doc][Followup] Revise the documentation of spark.sql.hive.metastore.jars #30407

Closed

xkrogen mentioned this pull request Jan 7, 2021

[SPARK-34044][DOCS] Add spark.sql.hive.metastore.jars.path to sql-data-sources-hive-tables.md #31085

Closed

AngersZhuuuu mentioned this pull request Jan 25, 2021

[SPARK-32852][SQL][FOLLOW_UP] Add notice about keep hive version consistence when config hive jars location #31317

Closed

[SPARK-32852][SQL] spark.sql.hive.metastore.jars support HDFS location #29881

[SPARK-32852][SQL] spark.sql.hive.metastore.jars support HDFS location #29881

Uh oh!

Conversation

AngersZhuuuu commented Sep 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

AngersZhuuuu commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Sep 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu commented Sep 27, 2020 •

edited

Loading

HyukjinKwon Sep 29, 2020 •

edited

Loading