[SPARK-16967] move mesos to module #14637

mgummelt · 2016-08-14T18:23:19Z

What changes were proposed in this pull request?

Move Mesos code into a mvn module

How was this patch tested?

unit tests
manually submitting a client mode and cluster mode job
spark/mesos integration test suite

jaceklaskowski · 2016-08-14T18:42:05Z

mesos/pom.xml

Isn't the line too long?

I don't think line length rules apply to XML. See the other poms for examples.

SparkQA · 2016-08-14T18:42:46Z

Test build #63753 has finished for PR 14637 at commit 57a191b.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

jaceklaskowski · 2016-08-14T18:43:27Z

mesos/pom.xml

Why do we need that commented-out section?

This is a WIP. Please wait to comment.

SparkQA · 2016-08-14T18:57:25Z

Test build #63754 has finished for PR 14637 at commit 465929e.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-08-15T00:07:28Z

Test build #63765 has finished for PR 14637 at commit 4e276ab.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2016-08-15T09:14:06Z

Directionally this is looking spot on, thank you. Ping when you want a review.

mgummelt · 2016-08-19T23:10:17Z

project/SparkBuild.scala

FYI I'm just pattern matching here to get -Pmesos to work with sbt. I have no idea what this does.

This is a pattern matching on assignment feature in Scala and the line creates one optionallyEnabledProjects value for the entire Seq of projects with respective projects being their own values mesos etc.

mgummelt · 2016-08-19T23:10:52Z

@srowen Ready for review.

SparkQA · 2016-08-19T23:28:59Z

Test build #64109 has finished for PR 14637 at commit 95e473b.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

mgummelt · 2016-08-19T23:59:36Z

mima seems to be upset about my removal of MESOS_REGEX from SparkMasterRegex, but I don't understand why, as it's a private class. Should I add an entry to MimaExcludes?

srowen · 2016-08-20T10:25:49Z

mesos/pom.xml

+      </exclusions>
+    </dependency>
+
+    <dependency>


I don't know that you need these dependencies -- literally don't know either way by looking. These are definitely needed? I was wondering whether jetty was relevant.

I just now removed mockito, because it was already a dependency of the parent project.

I'm pretty sure all of the shaded deps need to be listed. I don't know why they're shaded in the first place, but since they are, they must be listed here to avoid classloading errors at runtime.

You shouldn't need to touch the shaded dependencies (see how no other module does that). If you're running into problems, you're probably missing something else, so post the error so we can take a look.

YARN does it: https://github.com/apache/spark/blob/master/yarn/pom.xml#L79

which is what I was using as a starting point. I initially did try to remove them, but got a ClassNotFoundException in WebUI.class when trying to import org.eclipse.jetty.. I can try to repro and get more info if you need.

The parent project seems to rename org.eclipse.jetty to org.spark_project.jetty https://github.com/apache/spark/blob/master/pom.xml#L2273

It's documented that the shaded deps should be put into compile scope in the module: https://github.com/apache/spark/blob/master/pom.xml#L315

Though I still don't understand a) why they're shaded in the first place, and b) how those classes are loaded when spark is built without a module that promotes those dependencies to compile scope.

They're shaded because people want to use different versions of guava and jetty.

Spark doesn't need to promote them to compile scope because the relocated guava / jetty classes are packaged with Spark itself.

I see what they problem you're running into is now. It has nothing to do with compile scope, you just need to explicit list the dependencies because the mesos code references them directly. So you actually just need to fix the comment since you're not promoting anything to compile scope by explicitly adding the dependencies (they still inherit the scope from the parent pom, which is "provided", and is actually what you want here).

(Note to self: YARN - and this new mesos module - should only need to declare the Guava dependency explicitly, since they don't use Jetty directly.)

Ah, I didn't realize the intransitivity of provided dependencies applied to inheritance as well.

srowen · 2016-08-20T10:31:14Z

You can ignore the MiMa error and suppress it, yes.
This is generally looking great, thanks. @vanzin would be a good reviewer too.

I think we'll have to change the Jenkins job config later to set -Pmesos -- will see about it.
There are some other miscellaneous places that may need to add the new profile, dev/:

lint-java, scalastyle
mima add to the profiles it uses for MiMa
create-release/release-build.sh I assume the default tarballs will still build in Mesos

Although it's kind of dead config, you might add the profile to .travis.yml too.

vanzin · 2016-08-23T17:07:50Z

If this is ready for review could you remove "wip" from the pr title?

vanzin · 2016-08-23T17:14:23Z

Also, with the current changes, doesn't it mean the PR builder is not building the mesos code? Shouldn't you modify dev/run-tests.py (I think that's the right place)?

The modification to the dependency files also raises another question: how will this be made available to users? Will there be a new package "spark-with-mesos" that needs to be packaged separately? Will users add it with "--packages" instead? There might be changes needed to dev/create-release/release-build.sh depending on the answer.

srowen · 2016-08-23T17:22:36Z

Good call, I think it's dev/sparktests/modules.py that needs to be modified. it can clone YARN's config.

I assume that the release binaries will have this profile activated as with YARN.

mgummelt · 2016-08-23T18:16:24Z

@srowen OK, added the profile to:

.travis.yml
release-build.sh
lint-java
mima
scalastyle

What about dev/test-dependencies.sh.

Also, I added mesos to PUBLISH_PROFILES in release-build.sh. Should I also add it to the make_binary_release profiles?

srowen · 2016-08-23T18:24:24Z

Yes to all three of those.

SparkQA · 2016-08-23T18:37:43Z

Test build #64301 has finished for PR 14637 at commit e94f5e7.

This patch fails executing the dev/run-tests script.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-08-23T18:46:24Z

Test build #64302 has finished for PR 14637 at commit fe7d05e.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-08-23T20:33:19Z

Test build #64299 has finished for PR 14637 at commit 3e10acc.

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

SparkQA · 2016-08-23T23:02:47Z

Test build #64303 has finished for PR 14637 at commit 05614b4.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-08-23T23:05:54Z

Test build #64312 has finished for PR 14637 at commit cdc5753.

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

mgummelt · 2016-08-23T23:39:21Z

retest this please

SparkQA · 2016-08-24T01:33:19Z

Test build #64317 has finished for PR 14637 at commit cdc5753.

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

mgummelt · 2016-08-24T18:37:45Z

retest this please

SparkQA · 2016-08-24T21:10:03Z

Test build #64370 has finished for PR 14637 at commit cdc5753.

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

mgummelt · 2016-08-24T21:12:29Z

@srowen Ready for review

srowen · 2016-08-25T08:52:18Z

Nice one, LGTM. I'll leave it open for final comments until tomorrow.

vanzin · 2016-08-25T17:46:53Z

dev/create-release/release-build.sh

-  make_binary_release "without-hadoop" "-Psparkr -Phadoop-provided -Pyarn" "3038" &
+  FLAGS="-Psparkr -Phadoop-2.3 -Phive -Phive-thriftserver -Pyarn -Pmesos"
+  make_binary_release "hadoop2.3" "$FLAGS" "3033" &
+  make_binary_release "hadoop2.4" "$FLAGS" "3034" &


This is wrong now; "FLAGS" enables "-Phadoop-2.3" when here it shuold be "-Phadoop-2.4" (and matching versions in the lines below).

ah, yea. fixing...

vanzin · 2016-08-25T19:57:31Z

LGTM now.

SparkQA · 2016-08-25T20:25:51Z

Test build #64430 has finished for PR 14637 at commit 09f3197.

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

mgummelt · 2016-08-25T20:32:19Z

retest this please

SparkQA · 2016-08-25T22:53:48Z

Test build #64435 has finished for PR 14637 at commit 09f3197.

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

vanzin · 2016-08-26T19:25:04Z

Merging to master.

drcrallen · 2017-02-06T20:08:14Z

FYI, we have a build process that packages spark core, now that mesos is is in its own artifact, this broke our build and deploy process, and its not called out in release notes

srowen · 2017-02-06T20:15:16Z

The release notes are for end users, and this doesn't impact end users. Developers are expected, more or less, to follow commits and dev@ to keep up with changes like this.

mgummelt force-pushed the mesos-module branch 2 times, most recently from 57a191b to 465929e Compare August 14, 2016 18:37

jaceklaskowski reviewed Aug 14, 2016
View reviewed changes

mgummelt force-pushed the mesos-module branch from 465929e to 4e276ab Compare August 14, 2016 23:50

mgummelt force-pushed the mesos-module branch 2 times, most recently from 1e9a0db to 21dc917 Compare August 19, 2016 23:08

mgummelt reviewed Aug 19, 2016
View reviewed changes

move mesos to module

95e473b

mgummelt force-pushed the mesos-module branch from 21dc917 to 95e473b Compare August 19, 2016 23:10

srowen reviewed Aug 20, 2016
View reviewed changes

add profile to build instructions

3e10acc

mgummelt changed the title ~~[WIP] [SPARK-16967] move mesos to module~~ [SPARK-16967] move mesos to module Aug 23, 2016

added test module, test-dependencies.sh, and release-build.sh

e94f5e7

fix syntax erro

fe7d05e

replace mesos deps in dependency tests

05614b4

add back in mockito

cdc5753

vanzin reviewed Aug 25, 2016
View reviewed changes

fix flags

09f3197

asfgit closed this in 8e5475b Aug 26, 2016

mgummelt deleted the mesos-module branch September 6, 2016 21:15

[SPARK-16967] move mesos to module #14637

[SPARK-16967] move mesos to module #14637

Uh oh!

Conversation

mgummelt commented Aug 14, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 14, 2016

Uh oh!

SparkQA commented Aug 15, 2016

Uh oh!

srowen commented Aug 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgummelt commented Aug 19, 2016

Uh oh!

SparkQA commented Aug 19, 2016

Uh oh!

mgummelt commented Aug 19, 2016

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!

mgummelt Aug 23, 2016 • 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!

srowen commented Aug 20, 2016

Uh oh!

vanzin commented Aug 23, 2016

Uh oh!

vanzin commented Aug 23, 2016

Uh oh!

srowen commented Aug 23, 2016

Uh oh!

mgummelt commented Aug 23, 2016

Uh oh!

srowen commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 23, 2016

Uh oh!

mgummelt commented Aug 23, 2016

Uh oh!

SparkQA commented Aug 24, 2016

Uh oh!

mgummelt commented Aug 24, 2016

Uh oh!

SparkQA commented Aug 24, 2016

Uh oh!

mgummelt Aug 23, 2016 •

edited

Loading