[SPARK-12807] [YARN] Spark External Shuffle not working in Hadoop clusters with Jackson 2.2.3 by steveloughran · Pull Request #10780 · apache/spark

steveloughran · 2016-01-15T23:56:43Z

Patch to

Shade jackson 2.x in spark-yarn-shuffle JAR: core, databind, annotation
Use maven antrun to verify the JAR has the renamed classes

Being Maven-based, I don't know if the verification phase kicks in on an SBT/jenkins build. It will on a mvn install

steveloughran · 2016-01-16T01:28:44Z

Updated patch which has shading of com.fasterxml.jackson -> org.spark-project.com.fasterxml.jackson

There's a test for this working; it uses the ant plugin to probe the JAR for .class files. It works, and I've verified that it stops working if you don't have the classes in. This validation runs in the verify phase, immediately before the install action: you can't install this package without the shading working. Accordingly: its a regression test on the feature.

This test does not (obviously) verify that this fixes the conflict between YARN NM and Shuffle. This is going to need external tests

SparkQA · 2016-01-16T03:07:26Z

Test build #49508 has finished for PR 10780 at commit 94d68a5.

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

SparkQA · 2016-01-16T03:22:10Z

Test build #49510 has finished for PR 10780 at commit c365551.

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

steveloughran · 2016-01-16T04:53:59Z

Jenkins, retest this please

SparkQA · 2016-01-16T06:28:19Z

Test build #49522 has finished for PR 10780 at commit c365551.

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

steveloughran · 2016-01-19T20:59:03Z

The latest patch

relocates everything except leveldb. Leveldb uses JNI stuff, which doesn't relocate, and on JVMs with multiple classloaders, can still only be loaded once. Jackson, netty and javax.annotation all go under org.spark-project.
Added a leveldb-provided profile which excludes leveldb from both the shuffle and hadoop-hdfs dependencies. (The latter could be excluded in the root spark POM; it's server-side only).

When built with leveldb-provided, the relocation stops version conflict in simple java classes; leveldb will be needed to be added to the NM classpath —Hadoop 2.6+(?) certainly does that.

Maybe instead of leveldb-provided, the profile could just become hadoop-2.6, which is already in the top-level

SparkQA · 2016-01-19T21:53:13Z

Test build #49698 has finished for PR 10780 at commit 1813955.

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

SparkQA · 2016-01-20T02:56:22Z

Test build #49736 has finished for PR 10780 at commit 98f18aa.

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

SparkQA · 2016-02-03T22:18:31Z

Test build #50675 has finished for PR 10780 at commit 0ae0955.

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

vanzin · 2016-02-04T21:20:21Z

@steveloughran given the discussion on the bug, can you remove shading for anything but Jackson?

In particular, shading the javax annotations is wrong because that would probably break tools that rely on them to do checks.

steveloughran · 2016-02-05T10:18:27Z

Can do

steveloughran · 2016-02-05T12:47:05Z

I've just pushed out a new version which leaves only jackson and guava as the relocations: the ones that are most trouble. I could turn off the guava relocate, though that introduces a risk of the shuffle failing if the hadoop bundled guava 11.x gets picked up first. The hadoop codebase is designed to work with later guavas, so can cope with the spark one being picked up.

leveldb can be omitted with a Pleveldb-provided profile; this is disabled by default (i.e. leveldb is bundled)

SparkQA · 2016-02-05T15:08:57Z

Test build #50815 has finished for PR 10780 at commit 43d2b34.

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

vanzin · 2016-02-05T17:38:54Z

Why not use the shade property you created here too?

one is {{org/spark-project}}, the other {{org.spark-project}}; you'd have to do s/r renaming to automatically derive one from the other. With hindsight, it's a common enough use case we should have added it to Ant 15 years ago, but its too late now.

…verify shading is there. currently they fail

…right

…eed to be asserting state of JAR

…builds

…n-module-scala

…unshaded artifacts

… the artifact. the build fails, as the verify stage is still probing for it

…ffle versions being in perfect sync, so duplication is harmless

SparkQA · 2016-02-08T14:24:35Z

Test build #50918 has finished for PR 10780 at commit 947a798.

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

vanzin · 2016-02-08T17:22:52Z

+ <relocation>
+ <pattern>com.fasterxml.jackson</pattern>
+ <shadedPattern>org.spark-project.com.fasterxml.jackson</shadedPattern>
+ <includes>


super nit: I wonder if this is needed; we have it for Jetty but not for Guava, and both end up being relocated the same way.

maven shading is a dark mystery to me. I don't understand what it does; I don't trust it. What you see there is cargo-cult cut and paste backed by a bit of ant to verify the final outcome of the C&P matches what I want.

vanzin · 2016-02-08T17:23:38Z

LGTM.

vanzin · 2016-02-09T18:59:32Z

Merging this.

…ters with Jackson 2.2.3 Patch to 1. Shade jackson 2.x in spark-yarn-shuffle JAR: core, databind, annotation 2. Use maven antrun to verify the JAR has the renamed classes Being Maven-based, I don't know if the verification phase kicks in on an SBT/jenkins build. It will on a `mvn install` Author: Steve Loughran <stevel@hortonworks.com> Closes#10780 from steveloughran/stevel/patches/SPARK-12807-master-shuffle. (cherry picked from commit 34d0b70) Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>

steveloughran · 2016-02-10T13:57:26Z

thanks!

steveloughran changed the title ~~[SPARK-12807] [YARN] [WIP] Spark External Shuffle not working in Hadoop clusters with Jackson 2.2.3~~[SPARK-12807] [YARN] Spark External Shuffle not working in Hadoop clusters with Jackson 2.2.3Jan 16, 2016

steveloughran mentioned this pull request Jan 16, 2016
[SPARK-12807] [YARN] Spark External Shuffle not working in Hadoop clusters with Jackson 2.2.3: branch-1.6 patch #10782
Closed

steveloughran force-pushed the stevel/patches/SPARK-12807-master-shuffle branch from 98f18aa to 0ae0955Compare February 3, 2016 19:56

steveloughran force-pushed the stevel/patches/SPARK-12807-master-shuffle branch from 0ae0955 to 43d2b34Compare February 5, 2016 12:47

vanzin reviewed Feb 5, 2016
View reviewed changes

steveloughran added 11 commits February 8, 2016 11:38

SPARK-12807 WiP shading patch + integration tests (after shading) to …
4265afb
…verify shading is there. currently they fail

SPARK-12807 add log4j.properties test resource
2a876fe

SPARK-12807 IT test to use / paths. Moot, as tests classpaths aren't …
4221e73
…right

SPARK-12807 Switch to using Ant to scan internals of zipfile. Guarant…
35d478d
…eed to be asserting state of JAR

SPARK-12807 class check taking place in verify phase; works on clean …
a57853b
…builds

SPARK-12807 add probe for scala module
25144ec

SPARK-12807 cut explicit references for jackson
586fc79

SPARK-12807 tightened build: reduced diff to master, leave out jackso…
32980ea
…n-module-scala

SPARK-12807 macrodefing the build process, for scalability of checks
2e694aa

SPARK-12807 <shaded> extends <exists>, which can be used for probing …
f16cf13
…unshaded artifacts

SPARK-12807 add leveldb-provided profile, which excludes leveldb from…
7489f89
… the artifact. the build fails, as the verify stage is still probing for it

steveloughran added 4 commits February 8, 2016 11:38

SPARK-12807 cut leveldb check
f39275d

SPARK-12807 cull (broken) test methods obsoleted by build file checks
81f7304

[SPARK-12807] remove netty & javax.annotation from relocation
e8d4b3a

[SPARK-12807] remove all leveldb related work; rely on hadoop and shu…
947a798
…ffle versions being in perfect sync, so duplication is harmless

steveloughran force-pushed the stevel/patches/SPARK-12807-master-shuffle branch from 43d2b34 to 947a798Compare February 8, 2016 12:08

vanzin reviewed Feb 8, 2016
View reviewed changes

asfgit closed this in 34d0b70Feb 9, 2016

Uh oh!

Conversation

steveloughran commented Jan 15, 2016

Uh oh!

steveloughran commented Jan 16, 2016

Uh oh!

SparkQA commented Jan 16, 2016

Uh oh!

SparkQA commented Jan 16, 2016

Uh oh!

steveloughran commented Jan 16, 2016

Uh oh!

SparkQA commented Jan 16, 2016

Uh oh!

steveloughran commented Jan 19, 2016

Uh oh!

SparkQA commented Jan 19, 2016

Uh oh!

SparkQA commented Jan 20, 2016

Uh oh!

SparkQA commented Feb 3, 2016

Uh oh!

vanzin commented Feb 4, 2016

Uh oh!

steveloughran commented Feb 5, 2016

Uh oh!

steveloughran commented Feb 5, 2016

Uh oh!

SparkQA commented Feb 5, 2016

Uh oh!

vanzinFeb 5, 2016

Choose a reason for hiding this comment

Uh oh!

steveloughranFeb 8, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 8, 2016

Uh oh!

vanzinFeb 8, 2016

Choose a reason for hiding this comment

Uh oh!

steveloughranFeb 8, 2016

Choose a reason for hiding this comment

Uh oh!

vanzin commented Feb 8, 2016

Uh oh!

vanzin commented Feb 9, 2016

Uh oh!

steveloughran commented Feb 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants