Skip to content

Conversation

@pan3793
Copy link
Member

@pan3793 pan3793 commented Dec 9, 2022

What changes were proposed in this pull request?

Fix shading in core module.

Why are the changes needed?

Fixed issue mentioned in #38779 (comment)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

➜  dist git:(SPARK-41244-followup) bin/spark-shell
22/12/09 16:35:43 WARN Utils: Your hostname, Chengs-MacBook-Pro.local resolves to a loopback address: 127.0.0.1; using 10.221.96.92 instead (on interface en0)
22/12/09 16:35:43 WARN Utils: Set SPARK_LOCAL_IP if you need to bind to another address
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
22/12/09 16:35:46 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Spark context Web UI available at http://10.221.96.92:4040
Spark context available as 'sc' (master = local[*], app id = local-1670574946548).
Spark session available as 'spark'.
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 3.4.0-SNAPSHOT
      /_/

Using Scala version 2.12.17 (OpenJDK 64-Bit Server VM, Java 1.8.0_332)
Type in expressions to have them evaluated.
Type :help for more information.

scala> spark.sql("select 1").show
+---+
|  1|
+---+
|  1|
+---+

image

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-41244] Fix shading in core module [SPARK-41244][BUILD] Fix shading in core module Dec 9, 2022
@dongjoon-hyun
Copy link
Member

cc @gengliangwang , @LuciferYang @cloud-fan @mridulm @amaliujia @tgravescs , @ulysses-you

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-41244][BUILD] Fix shading in core module [SPARK-41244][BUILD][FOLLOWUP] Fix shading in core module Dec 9, 2022
@dongjoon-hyun
Copy link
Member

cc @parthchandra , too.

<artifactSet>
<includes>
<include>org.spark-project.spark:unused</include>
<include>org.eclipse.jetty:jetty-io</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

if we move this part to core/pom.xml, also need remove the relocation of jetty from parent pom.xml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly, jetty classes should be included into core only, but relocation is required for all modules, otherwise, other modules like sql reference the vanilla jetty classes but core ships relocated jetty classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it

<shadeTestJar>true</shadeTestJar>
<artifactSet>
<includes>
<include>org.spark-project.spark:unused</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you compared the compiled logs of Maven shade? If it is not use append mode, we need make sure there is no omission.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the result is expected.

This PR

[INFO] --- maven-shade-plugin:3.2.4:shade (default) @ spark-core_2.12 ---
[INFO] Including org.eclipse.jetty:jetty-plus:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-security:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-util:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-server:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-io:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-http:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-continuation:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlet:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-proxy:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-client:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlets:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including com.google.protobuf:protobuf-java:jar:3.21.9 in the shaded jar.
[INFO] Including org.spark-project.spark:unused:jar:1.0.0 in the shaded jar.

Before #38779

[INFO] --- maven-shade-plugin:3.2.4:shade (default) @ spark-core_2.12 ---
[INFO] Including org.eclipse.jetty:jetty-plus:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-security:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-util:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-server:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-io:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-http:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-continuation:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlet:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-proxy:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-client:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.eclipse.jetty:jetty-servlets:jar:9.4.49.v20220914 in the shaded jar.
[INFO] Including org.spark-project.spark:unused:jar:1.0.0 in the shaded jar.

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM.

@HyukjinKwon HyukjinKwon changed the title [SPARK-41244][BUILD][FOLLOWUP] Fix shading in core module [SPARK-41450][BUILD] Fix shading in core module Dec 9, 2022
@HyukjinKwon
Copy link
Member

let me change the JIRA in the title since there's already a JIRA open.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this.

@dongjoon-hyun
Copy link
Member

Merged to master. Thank you, @pan3793 and all!

@gengliangwang
Copy link
Member

@pan3793 Thanks for fixing it!

beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
### What changes were proposed in this pull request?

Fix shading in core module.

### Why are the changes needed?

Fixed issue mentioned in apache#38779 (comment)

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

```
➜  dist git:(SPARK-41244-followup) bin/spark-shell
22/12/09 16:35:43 WARN Utils: Your hostname, Chengs-MacBook-Pro.local resolves to a loopback address: 127.0.0.1; using 10.221.96.92 instead (on interface en0)
22/12/09 16:35:43 WARN Utils: Set SPARK_LOCAL_IP if you need to bind to another address
Setting default log level to "WARN".
To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
22/12/09 16:35:46 WARN NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
Spark context Web UI available at http://10.221.96.92:4040
Spark context available as 'sc' (master = local[*], app id = local-1670574946548).
Spark session available as 'spark'.
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 3.4.0-SNAPSHOT
      /_/

Using Scala version 2.12.17 (OpenJDK 64-Bit Server VM, Java 1.8.0_332)
Type in expressions to have them evaluated.
Type :help for more information.

scala> spark.sql("select 1").show
+---+
|  1|
+---+
|  1|
+---+

```
<img width="1604" alt="image" src="https://user-images.githubusercontent.com/26535726/206659907-95d95d7f-dce2-4a4f-9aef-e54fb4612aba.png">

Closes apache#38999 from pan3793/SPARK-41244-followup.

Authored-by: Cheng Pan <chengpan@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants