[SPARK-29022][SQL]Fix SparkSQLCLI can not add jars by AddJarCommand #25729

AngersZhuuuu · 2019-09-09T08:29:02Z

What changes were proposed in this pull request?

For issue mentioned in SPARK-29022
Spark SQL CLI can't use class as serde class in jars add by SQL ADD JAR.

When we create table with serde class contains by jar added by SQL 'ADD JAR'.
We can create table with serde class construct success since we call HiveClientImpl.createTable under withHiveState method, it will add clientLoader.classLoader to HiveClientImpl.state.getConf.classLoader.

Jars added by SQL ADD JAR will be add to

sparkSession.sharedState.jarClassLoader.
'HiveClientLoader.clientLoader.classLoader'

In Current spark-sql MODE, HiveClientImpl.state will use CliSessionState created when initialize
SparkSQLCliDriver, When we select data from table, it will check serde class, when call method HiveTableScanExec#addColumnMetadataToConf() to check for table desc serde class.

val deserializer = tableDesc.getDeserializerClass.getConstructor().newInstance()
    deserializer.initialize(hiveConf, tableDesc.getProperties)

getDeserializer will use CliSessionState's hiveConf's classLoader in Spark SQL CLI mode.
But when we call ADD JAR in spark, the jar won't be added to Classloader of CliSessionState' conf , then ClassNotFound error happen.

So we reset CliSessionState conf's classLoader to sharedState.jarClassLoader when sharedState.jarClassLoader has added jar passed by HIVEAUXJARS
Then when we use ADD JAR to add jar, jar path will be added to CliSessionState's conf's ClassLoader

Why are the changes needed?

Fix bug

Does this PR introduce any user-facing change?

No

How was this patch tested?

ADD UT

dongjoon-hyun · 2019-09-09T17:14:01Z

Hi, @AngersZhuuuu . Is the JIRA issue ID correct? SPARK-29051 ?

AngersZhuuuu · 2019-09-09T23:06:31Z

Hi, @AngersZhuuuu . Is the JIRA issue ID correct? SPARK-29051 ?

Sorry, I forgot to change it. This PR should wait for #25542

This reverts commit c3de557.

wangyum · 2019-09-13T03:40:21Z

ok to test

SparkQA · 2019-09-13T04:08:09Z

Test build #110556 has finished for PR 25729 at commit 9d347f6.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
test(\"SPARK-29022 Use add jar class as serde\")

dongjoon-hyun · 2019-09-13T04:55:33Z

Retest this please.

dongjoon-hyun · 2019-09-13T04:56:16Z

hadoop-2.7 is tested. I triggered hadoop-3.2. After a few minutes later, I will trigger [test-hadoop3.2][test-java11], too.

AngersZhuuuu · 2019-09-13T05:05:01Z

hadoop-2.7 is tested. I triggered hadoop-3.2. After a few minutes later, I will trigger [test-hadoop3.2][test-java11], too.

Thanks.

dongjoon-hyun · 2019-09-13T05:06:08Z

@AngersZhuuuu . You need to change the title before triggering. 😄

SparkQA · 2019-09-13T05:24:34Z

Test build #110559 has finished for PR 25729 at commit 9d347f6.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
test(\"SPARK-29022 Use add jar class as serde\")

AngersZhuuuu · 2019-09-13T05:33:26Z

@AngersZhuuuu . You need to change the title before triggering. 😄

I intend to change title when build finish. Forgot that I can just change it when you have triggered a
build process.
You have change it. Maybe you should trigger build again for hadoop3.2 & java11.

dongjoon-hyun · 2019-09-13T06:14:17Z

Retest this please.

SparkQA · 2019-09-13T06:51:38Z

Test build #110562 has finished for PR 25729 at commit 9d347f6.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
test(\"SPARK-29022 Use add jar class as serde\")

wangyum · 2019-09-16T13:52:47Z

@dongjoon-hyun @srowen Maybe we can fix this issue by another PR: #25775

srowen · 2019-09-16T13:57:23Z

OK so this should resolve the same issue? are there arguments for against this change vs the other?

AngersZhuuuu · 2019-09-16T15:38:51Z

@srowen @wangyum
I have add a new UT for this issue to cover case use hive.aux.jars jar's class as serde.
#25775 can't solve problem of this issue.

@dongjoon-hyun we may need to retest all case again

SparkQA · 2019-09-16T19:53:34Z

Test build #110645 has finished for PR 25729 at commit ce45394.

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

SparkQA · 2019-09-17T00:18:15Z

Test build #110673 has finished for PR 25729 at commit 1d60b90.

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

SparkQA · 2019-09-17T03:31:25Z

Test build #110714 has finished for PR 25729 at commit 5486f75.

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

…nto SPARK-29015

SparkQA · 2019-09-29T04:45:47Z

Test build #111553 has finished for PR 25729 at commit d070059.

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

SparkQA · 2019-09-29T04:52:35Z

Test build #111554 has finished for PR 25729 at commit fcfd7af.

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

SparkQA · 2019-09-29T11:51:00Z

Test build #111565 has finished for PR 25729 at commit db63cf2.

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

AngersZhuuuu · 2019-09-29T13:53:56Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala

+        // In HiveThriftServer2, it will call HiveUtils.newClientForExecution() to get a client
+        // for Execution, then that method will trigger here to execute, in that case we can't reset
+        // ret.getConf's ClassLoader.
+        if (HiveUtils.isCliSessionState) {


@srowen @wangyum

Some special point for jdk11. Need to add more explanation？

SparkQA · 2019-09-29T15:29:33Z

Test build #111568 has finished for PR 25729 at commit 918941e.

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

SparkQA · 2019-09-30T02:16:10Z

Test build #111583 has finished for PR 25729 at commit 72d4d84.

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

AngersZhuuuu · 2019-09-30T06:21:04Z

@dongjoon-hyun
Passed jdk11 test, I have changed the title, thanks for trigger build for jdk8 & hadoop3.2.

wangyum · 2019-09-30T06:24:10Z

retest this please

SparkQA · 2019-09-30T06:54:27Z

Test build #111598 has finished for PR 25729 at commit 72d4d84.

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

wangyum · 2019-09-30T06:59:23Z

retest this please

SparkQA · 2019-09-30T07:27:48Z

Test build #111599 has finished for PR 25729 at commit 72d4d84.

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

srowen

Looks plausible. @wangyum ?

wangyum

Thank you for ping me @srowen. The change LGTM.

srowen · 2019-10-01T15:10:21Z

Merged to master

dongjoon-hyun added the SQL label Sep 9, 2019

AngersZhuuuu changed the title ~~[WIP][SPARK-29051][SQL] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [WIP][SPARK-29022][SQL] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 9, 2019

AngersZhuuuu added 4 commits September 10, 2019 17:44

save change

c3de557

Revert "save change"

2cf3153

This reverts commit c3de557.

Merge remote-tracking branch 'upstream/master'

e5cd06c

Merge remote-tracking branch 'upstream/master'

569ab8a

AngersZhuuuu changed the title ~~[WIP][SPARK-29022][SQL] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 13, 2019

AngersZhuuuu changed the title ~~[SPARK-29022][SQL] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL][test-hadoop3.2] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 13, 2019

AngersZhuuuu closed this Sep 13, 2019

AngersZhuuuu reopened this Sep 13, 2019

dongjoon-hyun changed the title ~~[SPARK-29022][SQL][test-hadoop3.2] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL][test-hadoop3.2][test-java11] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 13, 2019

srowen approved these changes Sep 16, 2019

View reviewed changes

AngersZhuuuu added 5 commits September 29, 2019 12:07

merge master

c108eca

Merge branch 'SPARK-29015' of https://github.com/AngersZhuuuu/spark i…

d070059

…nto SPARK-29015

Merge branch 'master' into SPARK-29015

e8326a7

Use new method

8633faf

Merge branch 'SPARK-29015' of https://github.com/AngersZhuuuu/spark i…

fcfd7af

…nto SPARK-29015

use right jar

db63cf2

Only reset classloader when cli mode

918941e

AngersZhuuuu commented Sep 29, 2019

View reviewed changes

fix in SparkSQLCliDriver

72d4d84

AngersZhuuuu changed the title ~~[SPARK-29022][SQL][test-hadoop3.2][test-java11] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL][test-hadoop3.2] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 30, 2019

AngersZhuuuu changed the title ~~[SPARK-29022][SQL][test-hadoop3.2] Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL]Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound Sep 30, 2019

srowen reviewed Oct 1, 2019

View reviewed changes

AngersZhuuuu changed the title ~~[SPARK-29022][SQL]Fix spark 'add jar', CliSessionState's hiveConf 's classLoader ClassNotFound~~ [SPARK-29022][SQL] Reset CliSessionState.conf's ClassLoader to SharedState.jarClassLoader after initialize SparkSQLCliDriver and handle HIVEAUXJARS Oct 1, 2019

AngersZhuuuu changed the title ~~[SPARK-29022][SQL] Reset CliSessionState.conf's ClassLoader to SharedState.jarClassLoader after initialize SparkSQLCliDriver and handle HIVEAUXJARS~~ [SPARK-29022][SQL]Fix SparkSQLCLI can not add jars by AddJarCommand Oct 1, 2019

wangyum approved these changes Oct 1, 2019

View reviewed changes

srowen closed this in 0cf2f48 Oct 1, 2019

[SPARK-29022][SQL]Fix SparkSQLCLI can not add jars by AddJarCommand #25729

[SPARK-29022][SQL]Fix SparkSQLCLI can not add jars by AddJarCommand #25729

Uh oh!

Conversation

AngersZhuuuu commented Sep 9, 2019 • 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!

dongjoon-hyun commented Sep 9, 2019

Uh oh!

AngersZhuuuu commented Sep 9, 2019

Uh oh!

wangyum commented Sep 13, 2019

Uh oh!

SparkQA commented Sep 13, 2019

Uh oh!

dongjoon-hyun commented Sep 13, 2019

Uh oh!

dongjoon-hyun commented Sep 13, 2019

Uh oh!

AngersZhuuuu commented Sep 13, 2019

Uh oh!

dongjoon-hyun commented Sep 13, 2019

Uh oh!

SparkQA commented Sep 13, 2019

Uh oh!

AngersZhuuuu commented Sep 13, 2019

Uh oh!

dongjoon-hyun commented Sep 13, 2019

Uh oh!

SparkQA commented Sep 13, 2019

Uh oh!

wangyum commented Sep 16, 2019

Uh oh!

srowen commented Sep 16, 2019

Uh oh!

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

Uh oh!

Uh oh!

SparkQA commented Sep 16, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

SparkQA commented Sep 17, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

AngersZhuuuu Sep 29, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 29, 2019

Uh oh!

SparkQA commented Sep 30, 2019

Uh oh!

AngersZhuuuu commented Sep 30, 2019

Uh oh!

wangyum commented Sep 30, 2019

Uh oh!

SparkQA commented Sep 30, 2019

Uh oh!

wangyum commented Sep 30, 2019

Uh oh!

SparkQA commented Sep 30, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

wangyum left a comment

Choose a reason for hiding this comment

Uh oh!

srowen commented Oct 1, 2019

Uh oh!

Reviewers

AngersZhuuuu commented Sep 9, 2019 •

edited

Loading

AngersZhuuuu commented Sep 16, 2019 •

edited

Loading