[SPARK-25694][SQL] Add a config for `URL.setURLStreamHandlerFactory` by jiangzho · Pull Request #26530 · apache/spark

jiangzho · 2019-11-14T19:06:57Z

What changes were proposed in this pull request?

Add a property spark.fsUrlStreamHandlerFactory.enabled to allow users turn off the default registration of org.apache.hadoop.fs.FsUrlStreamHandlerFactory

Why are the changes needed?

This SPARK-25694 is a long-standing issue. Originally, [SPARK-12868][SQL] Allow adding jars from hdfs added this for better Hive support. However, this have a side-effect when the users use Apache Spark without -Phive. This causes exceptions when the users tries to use another custom factories or 3rd party library (trying to set this). This configuration will unblock those non-hive users.

Does this PR introduce any user-facing change?

Yes. This provides a new user-configurable property.
By default, the behavior is unchanged.

How was this patch tested?

Manual testing.

BEFORE

$ build/sbt package
$ bin/spark-shell
scala> sql("show tables").show
+--------+---------+-----------+
|database|tableName|isTemporary|
+--------+---------+-----------+
+--------+---------+-----------+
scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory())
java.lang.Error: factory already defined
at java.net.URL.setURLStreamHandlerFactory(URL.java:1134)
... 47 elided

AFTER

$ build/sbt package
$ bin/spark-shell --conf spark.sql.defaultUrlStreamHandlerFactory.enabled=false
scala> sql("show tables").show
+--------+---------+-----------+
|database|tableName|isTemporary|
+--------+---------+-----------+
+--------+---------+-----------+
scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory())

dongjoon-hyun · 2019-11-14T20:58:43Z

ok to test

dongjoon-hyun · 2019-11-14T21:00:45Z

Thank you for making your first PR, @jiangzho .

dongjoon-hyun · 2019-11-15T05:03:56Z

Retest this please.

dongjoon-hyun · 2019-11-15T16:52:24Z

Retest this please

dbtsai · 2019-11-17T21:10:05Z

Add @steveloughran for more feedback.

…URLConnection issue

dongjoon-hyun · 2019-11-17T21:33:33Z

Hi, @srowen , @dbtsai .
This PR is rebased and updated according to the review comments.
Now we have a renamed spark.sql.defaultUrlStreamHandlerFactory.enabled config.
Could you review this once more?

SparkQA · 2019-11-18T03:14:30Z

Test build #113970 has finished for PR 26530 at commit 4367b15.

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

SparkQA · 2019-11-18T03:31:56Z

Test build #113971 has finished for PR 26530 at commit 558062b.

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

dongjoon-hyun · 2019-11-18T04:20:07Z

Now, AtomicBoolean is reverted~

dbtsai

LGTM.

srowen

OK by me. I think the checking outside the synchronized block isn't that important here as there won't be contention, but this is OK.

dongjoon-hyun · 2019-11-18T04:33:23Z

Thank you for review and approval, @dbtsai and @srowen . I really want to put this into Apache Spark master. :)

SparkQA · 2019-11-18T05:10:40Z

Test build #113974 has finished for PR 26530 at commit d9e0ec7.

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

SparkQA · 2019-11-18T05:31:15Z

Test build #113976 has finished for PR 26530 at commit c4ec2d5.

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

dbtsai · 2019-11-18T05:44:42Z

Merged into master. Thanks all!

SparkQA · 2019-11-18T06:11:33Z

Test build #113979 has finished for PR 26530 at commit cd9ed48.

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

HyukjinKwon · 2019-11-18T06:21:57Z

+
+package object config {
+
+ private[spark] val DEFAULT_URL_STREAM_HANDLER_FACTORY_ENABLED =


Why is it in a separate file not in StaticSQLConf or SQLConf?

Made a followup - #26570

dongjoon-hyun · 2019-11-18T07:39:28Z

Thank you for merging, @dbtsai . Thanks for the follow-up, @HyukjinKwon .

@jiangzho . You are added to the Apache Spark Contributor group.
https://issues.apache.org/jira/browse/SPARK-25694 is assigned to you.
Thank you for making this PR.

SparkQA · 2019-11-18T08:05:02Z

Test build #113985 has finished for PR 26530 at commit 755c9c0.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

…Factory.enabled' into StaticSQLConf.scala ### What changes were proposed in this pull request? This PR is a followup of #26530 and proposes to move the configuration `spark.sql.defaultUrlStreamHandlerFactory.enabled` to `StaticSQLConf.scala` for consistency. ### Why are the changes needed? To put the similar configurations together and for readability. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Manually tested as described in #26530. Closes#26570 from HyukjinKwon/SPARK-25694. Authored-by: HyukjinKwon <gurwls223@apache.org> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

steveloughran · 2019-11-27T14:29:23Z

Sorry, missed this. I don't think the Hadoop HTTP FS should be registering its classes as the handlers of HTTP/HTTPS requests at all, as it breaks all the code which expects to cast it to well known types.

HADOOP-14598 fixes it for the https connections -really people should be using a version of the hadoop libraries with spark. These are old, fixed, bugs you are trying to work around here.

dongjoon-hyun · 2019-11-27T15:27:39Z

Thank you, @steveloughran . Sure.
Since HADOOP-14598 is Hadoop 2.9.0+, we will remove the legacy setURLStreamHanderFactory after we upgrade from Hadoop 2.7.x to Hadoop 2.9+ someday.

dongjoon-hyun · 2020-05-18T23:51:20Z

Hi, @jiangzho , @dbtsai , @holdenk and all.
There exists an on-going bug report and fix for setURLStreamHandlerFactory.

[SPARK-31692][SQL][2.4] Pass hadoop confs specifed via Spark confs to URLStreamHandlerfactory #28529 .

This PR is also helpful and safe. I'll backport this to branch-2.4.

Add a property `spark.fsUrlStreamHandlerFactory.enabled` to allow users turn off the default registration of `org.apache.hadoop.fs.FsUrlStreamHandlerFactory` This [SPARK-25694](https://issues.apache.org/jira/browse/SPARK-25694) is a long-standing issue. Originally, [[SPARK-12868][SQL] Allow adding jars from hdfs](#17342 ) added this for better Hive support. However, this have a side-effect when the users use Apache Spark without `-Phive`. This causes exceptions when the users tries to use another custom factories or 3rd party library (trying to set this). This configuration will unblock those non-hive users. Yes. This provides a new user-configurable property. By default, the behavior is unchanged. Manual testing. **BEFORE** ``` $ build/sbt package $ bin/spark-shell scala> sql("show tables").show +--------+---------+-----------+ |database|tableName|isTemporary| +--------+---------+-----------+ +--------+---------+-----------+ scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory()) java.lang.Error: factory already defined at java.net.URL.setURLStreamHandlerFactory(URL.java:1134) ... 47 elided ``` **AFTER** ``` $ build/sbt package $ bin/spark-shell --conf spark.sql.defaultUrlStreamHandlerFactory.enabled=false scala> sql("show tables").show +--------+---------+-----------+ |database|tableName|isTemporary| +--------+---------+-----------+ +--------+---------+-----------+ scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory()) ``` Closes#26530 from jiangzho/master. Lead-authored-by: Zhou Jiang <zhou_jiang@apple.com> Co-authored-by: Dongjoon Hyun <dhyun@apple.com> Co-authored-by: zhou-jiang <zhou_jiang@apple.com> Signed-off-by: DB Tsai <d_tsai@apple.com> (cherry picked from commit ee3bd6d) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…Factory.enabled' into StaticSQLConf.scala This PR is a followup of #26530 and proposes to move the configuration `spark.sql.defaultUrlStreamHandlerFactory.enabled` to `StaticSQLConf.scala` for consistency. To put the similar configurations together and for readability. No. Manually tested as described in #26530. Closes#26570 from HyukjinKwon/SPARK-25694. Authored-by: HyukjinKwon <gurwls223@apache.org> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit 8469614) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

Add a property `spark.fsUrlStreamHandlerFactory.enabled` to allow users turn off the default registration of `org.apache.hadoop.fs.FsUrlStreamHandlerFactory` This [SPARK-25694](https://issues.apache.org/jira/browse/SPARK-25694) is a long-standing issue. Originally, [[SPARK-12868][SQL] Allow adding jars from hdfs](apache#17342 ) added this for better Hive support. However, this have a side-effect when the users use Apache Spark without `-Phive`. This causes exceptions when the users tries to use another custom factories or 3rd party library (trying to set this). This configuration will unblock those non-hive users. Yes. This provides a new user-configurable property. By default, the behavior is unchanged. Manual testing. **BEFORE** ``` $ build/sbt package $ bin/spark-shell scala> sql("show tables").show +--------+---------+-----------+ |database|tableName|isTemporary| +--------+---------+-----------+ +--------+---------+-----------+ scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory()) java.lang.Error: factory already defined at java.net.URL.setURLStreamHandlerFactory(URL.java:1134) ... 47 elided ``` **AFTER** ``` $ build/sbt package $ bin/spark-shell --conf spark.sql.defaultUrlStreamHandlerFactory.enabled=false scala> sql("show tables").show +--------+---------+-----------+ |database|tableName|isTemporary| +--------+---------+-----------+ +--------+---------+-----------+ scala> java.net.URL.setURLStreamHandlerFactory(new org.apache.hadoop.fs.FsUrlStreamHandlerFactory()) ``` Closesapache#26530 from jiangzho/master. Lead-authored-by: Zhou Jiang <zhou_jiang@apple.com> Co-authored-by: Dongjoon Hyun <dhyun@apple.com> Co-authored-by: zhou-jiang <zhou_jiang@apple.com> Signed-off-by: DB Tsai <d_tsai@apple.com> (cherry picked from commit ee3bd6d) [SPARK-25694][SQL][FOLLOW-UP] Move 'spark.sql.defaultUrlStreamHandlerFactory.enabled' into StaticSQLConf.scala This PR is a followup of apache#26530 and proposes to move the configuration `spark.sql.defaultUrlStreamHandlerFactory.enabled` to `StaticSQLConf.scala` for consistency. To put the similar configurations together and for readability. No. Manually tested as described in apache#26530. Closesapache#26570 from HyukjinKwon/SPARK-25694. Authored-by: HyukjinKwon <gurwls223@apache.org> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit 8469614) RB=1906279 BUG=LIHADOOP-50577 G=superfriends-reviewers R=zolin,yezhou,chsingh,mshen,fli,latang A=chsingh

dongjoon-hyun changed the title ~~[SPARK-25694][SQL]URL.setURLStreamHandlerFactory causing incompatible HttpURLConnection issue~~[SPARK-25694][SQL] Add a config for URL.setURLStreamHandlerFactoryNov 14, 2019

This comment has been minimized.
Sign in to view

srowen reviewed Nov 15, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

dongjoon-hyun added SPARK CORE SQL labels Nov 15, 2019

This comment has been minimized.
Sign in to view

jiangzhoand others added 3 commits November 17, 2019 13:30

[SPARK-25694]URL.setURLStreamHandlerFactory causing incompatible Http…
5d81e21
…URLConnection issue

Add double check locking for UrlStreamHandlerFactory.
1e4d5c1

Address comments
1970254

dongjoon-hyun force-pushed the master branch from 7eb9697 to 1970254Compare November 17, 2019 21:32

srowen reviewed Nov 17, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

simplify
4367b15

srowen reviewed Nov 17, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated
Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

Use NonFatal
558062b

This comment has been minimized.
Sign in to view

dongjoon-hyun added 2 commits November 17, 2019 17:19

Address comments
346ae61

preserve the behavior
d9e0ec7

srowen reviewed Nov 18, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

dongjoon-hyun reviewed Nov 18, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala

Use AtomicBoolean
c4ec2d5

dbtsai reviewed Nov 18, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

srowen reviewed Nov 18, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

srowen reviewed Nov 18, 2019
View reviewed changes

Comment threadsql/core/src/main/scala/org/apache/spark/sql/internal/SharedState.scala Outdated

fix
cd9ed48

srowen approved these changes Nov 18, 2019
View reviewed changes

Revert AtomicBoolean
755c9c0

dbtsai approved these changes Nov 18, 2019
View reviewed changes

srowen reviewed Nov 18, 2019
View reviewed changes

dbtsai closed this in ee3bd6dNov 18, 2019

HyukjinKwon reviewed Nov 18, 2019
View reviewed changes

HyukjinKwon mentioned this pull request Nov 18, 2019
[SPARK-25694][SQL][FOLLOW-UP] Move 'spark.sql.defaultUrlStreamHandlerFactory.enabled' into StaticSQLConf.scala #26570
Closed

wajda mentioned this pull request Jun 26, 2020
Unable to run Spline in yarn cluster mode unable to access producer url AbsaOSS/spline-spark-agent#102
Closed


		package object config {

		private[spark] val DEFAULT_URL_STREAM_HANDLER_FACTORY_ENABLED =

Uh oh!

Conversation

jiangzho commented Nov 14, 2019• edited by dongjoon-hyun 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!

dongjoon-hyun commented Nov 14, 2019

Uh oh!

dongjoon-hyun commented Nov 14, 2019

Uh oh!

This comment has been minimized.

Uh oh!

dongjoon-hyun commented Nov 15, 2019

Uh oh!

This comment has been minimized.

dongjoon-hyun commented Nov 15, 2019

Uh oh!

This comment has been minimized.

dbtsai commented Nov 17, 2019

Uh oh!

dongjoon-hyun commented Nov 17, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

dongjoon-hyun commented Nov 18, 2019

Uh oh!

dbtsai left a comment

Choose a reason for hiding this comment

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Nov 18, 2019

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

dbtsai commented Nov 18, 2019

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

HyukjinKwonNov 18, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwonNov 18, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Nov 18, 2019

Uh oh!

SparkQA commented Nov 18, 2019

Uh oh!

steveloughran commented Nov 27, 2019

Uh oh!

dongjoon-hyun commented Nov 27, 2019

Uh oh!

dongjoon-hyun commented May 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jiangzho commented Nov 14, 2019•
edited by dongjoon-hyun
Loading