[SPARK-16637] Unified containerizer #14275

mgummelt · 2016-07-20T01:18:16Z

What changes were proposed in this pull request?

New config var: spark.mesos.docker.containerizer={"mesos","docker" (default)}

This adds support for running docker containers via the Mesos unified containerizer: http://mesos.apache.org/documentation/latest/container-image/

The benefit is losing the dependency on dockerd, and all the costs which it incurs.

I've also updated the supported Mesos version to 0.28.2 for support of the required protobufs.

This is blocked on: #14167

How was this patch tested?

manually testing jobs submitted with both "mesos" and "docker" settings for the new config var.
spark/mesos integration test suite

mgummelt · 2016-07-20T01:18:23Z

cc @skonto

actually hold off until #14167 goes through.

mgummelt · 2016-07-20T01:19:20Z

core/src/main/scala/org/apache/spark/SparkConf.scala

Added Serializable so that SparkConf can be persisted to ZK

Is SparkConf persisted by a MesosClusterPersistenceEngineFactory instance or somewhere else?

Yea: https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterPersistenceEngine.scala#L111

Interesting, this wasn't already serializable eh. Normally that might be a significant choice but this just contains a concurrent map, so seems OK. (And this field avroNamespace which should have been in the object)

SparkQA · 2016-07-20T01:25:24Z

Test build #62569 has finished for PR 14275 at commit e0fe773.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Serializable

skonto · 2016-07-21T09:30:44Z

core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala

I think this should be: spark.mesos.containerizer. Otherwise its confusing. I guess you mean what is the containerizer for the docker image.

skonto · 2016-07-21T11:36:56Z

I tested it against our test suite it works fine. Have you tested it with an image: spark.mesos.executor.docker.image? Do you have a sample image? I would like to test it... we could add it to our suite as well...

skonto · 2016-07-21T11:43:35Z

core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala

Bridge mode is not supported right? Why we need that?
I read in the docs spark.mesos.executor.docker.portmaps is used for that. I think docs need update.
There was a question here in the user list (we should answer i think):
https://mail-archives.apache.org/mod_mbox/spark-user/201604.mbox/%3CCAKOFcwroULK_b2RajeAO3v_DZ2qS8uN86HekxUJejQA4=FspSw@mail.gmail.com%3E

Yea, this is effectively dead. I'll leave it to a future PR to remove.

Ok cool we clarified that, there is a lot of confusion out there. How about the image is there one i can use to test it quickly? I really want to do that before reporting its good.

You can use ours: https://github.com/mesosphere/universe/blob/version-3.x/repo/packages/S/spark/14/resource.json#L5

The Spark dist is located at /opt/spark/dist

I've also already tested this manually.

👍 ok just a quick check i will do (a ritual for me) and will be fine from my side and then i will invite the committers for inspection and merge (tom).

SparkQA · 2016-07-21T18:46:33Z

Test build #62684 has finished for PR 14275 at commit 53d01ec.

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

mgummelt · 2016-07-21T18:48:51Z

@skonto rebased and ready for re-review

SparkQA · 2016-07-21T18:51:45Z

Test build #62685 has finished for PR 14275 at commit c02b7c7.

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

SparkQA · 2016-07-21T20:25:53Z

Test build #62686 has finished for PR 14275 at commit ebaef05.

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

skonto · 2016-07-21T21:00:34Z

Test "supports spark.mesos.driverEnv.*" fails.
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62686/testReport/junit/org.apache.spark.scheduler.cluster.mesos/MesosClusterSchedulerSuite/_/

The reason is that the map there contains ".TEST_ENV", the dot is the issue.
s/spark.mesos.driverEnv/spark.mesos.driverEnv for prefix at line 365, MesosClusterScheduler.scala

mgummelt · 2016-07-21T21:46:20Z

good eye. fixed.

SparkQA · 2016-07-21T23:56:26Z

Test build #62694 has finished for PR 14275 at commit 085e228.

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

SparkQA · 2016-07-22T00:02:28Z

Test build #62695 has finished for PR 14275 at commit 5bb1a35.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Serializable

skonto · 2016-07-22T11:38:19Z

@srowen Could you pls check and merge?

srowen · 2016-07-22T12:05:17Z

core/src/main/scala/org/apache/spark/deploy/mesos/MesosDriverDescription.scala

Why can't this be val BTW?

probably because its not used outside the class from what i recall. There is no reason why it cant bw.

I'm probably missing your point but would that affect val? it might be private too but this change just removes val.

Removing val means schedulerProperties is not saved as a field of the class. It's just used during construction. I could also make this private val if that's more standard in this project.

That was my point too above. No public getters generated if no val is used and this can be justified if the field is not used in general outside of the class. I would choose for example private val if I need access to the private val of an instance of a class T within some other instance of T. The latter is not possible if I just use the field without val.

Oh right, this is what causes it to become a field. Yeah I suspect lots and lots of things in the code should not even be val.

mgummelt · 2016-07-22T17:41:58Z

@srowen ready for re-review

SparkQA · 2016-07-22T19:36:03Z

Test build #62735 has finished for PR 14275 at commit be145c7.

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

skonto · 2016-07-23T21:30:54Z

LGTM.

srowen · 2016-07-24T10:15:29Z

OK, looks good to me too though I don't really know about Mesos.

tnachen · 2016-07-25T02:37:47Z

core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerBackendUtil.scala

Can we have a sensible message/exception when we pass in a unknown containerizer?

srowen · 2016-07-25T19:17:23Z

Looks like one more last comment, and needs a rebase after #13051

SparkQA · 2016-07-25T20:50:16Z

Test build #62840 has finished for PR 14275 at commit 44c50d4.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-07-25T20:54:41Z

Test build #62841 has finished for PR 14275 at commit 4a0af2e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

mgummelt · 2016-07-27T20:36:47Z

Rebased and upgraded to Mesos 1.0.0 rather than 0.28.2, since

a) it's out now!, and
b) we need support for unified containerizer caching

FYI @skonto

Ready to merge @srowen

SparkQA · 2016-07-27T20:40:19Z

Test build #62937 has finished for PR 14275 at commit 82359ba.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Serializable

SparkQA · 2016-07-27T22:44:34Z

Test build #62939 has finished for PR 14275 at commit ec6707e.

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

skonto · 2016-07-29T11:56:05Z

@srowen Could you please merge?

srowen · 2016-07-29T12:51:00Z

Merged to master

mgummelt reviewed Jul 20, 2016
View reviewed changes

skonto reviewed Jul 21, 2016
View reviewed changes

mgummelt force-pushed the unified-containerizer branch from e0fe773 to 53d01ec Compare July 21, 2016 18:40

mgummelt force-pushed the unified-containerizer branch from 085e228 to 5bb1a35 Compare July 21, 2016 21:48

srowen reviewed Jul 22, 2016
View reviewed changes

tnachen reviewed Jul 25, 2016
View reviewed changes

unified containerizer

82359ba

mgummelt force-pushed the unified-containerizer branch from 4a0af2e to 82359ba Compare July 27, 2016 20:35

fix deps

ec6707e

asfgit closed this in 266b92f Jul 29, 2016

mgummelt deleted the unified-containerizer branch August 9, 2016 22:38

[SPARK-16637] Unified containerizer #14275

[SPARK-16637] Unified containerizer #14275

Uh oh!

Conversation

mgummelt commented Jul 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

mgummelt commented Jul 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto Jul 21, 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!

SparkQA commented Jul 20, 2016

Uh oh!

skonto Jul 21, 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!

skonto commented Jul 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skonto Jul 21, 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!

skonto Jul 22, 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!

SparkQA commented Jul 21, 2016

Uh oh!

mgummelt commented Jul 21, 2016

Uh oh!

SparkQA commented Jul 21, 2016

Uh oh!

SparkQA commented Jul 21, 2016

Uh oh!

skonto commented Jul 21, 2016

Uh oh!

mgummelt commented Jul 21, 2016

Uh oh!

SparkQA commented Jul 21, 2016

Uh oh!

SparkQA commented Jul 22, 2016

Uh oh!

skonto commented Jul 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto Jul 22, 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!

skonto Jul 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!

mgummelt commented Jul 22, 2016

mgummelt commented Jul 20, 2016 •

edited

Loading

mgummelt commented Jul 20, 2016 •

edited

Loading

skonto Jul 21, 2016 •

edited

Loading

skonto Jul 21, 2016 •

edited

Loading

skonto commented Jul 21, 2016 •

edited

Loading

skonto Jul 21, 2016 •

edited

Loading

skonto Jul 22, 2016 •

edited

Loading

skonto Jul 22, 2016 •

edited

Loading

skonto Jul 23, 2016 •

edited

Loading