[SPARK-15271] [MESOS] Allow force pulling executor docker images #13051

philipphoffmann · 2016-05-11T14:39:33Z

What changes were proposed in this pull request?

Mesos agents by default will not pull docker images which are cached
locally already. In order to run Spark executors from mutable tags like
:latest this commit introduces a Spark setting
spark.mesos.executor.docker.forcePullImage. Setting this flag to
true will tell the Mesos agent to force pull the docker image (default is false which is consistent with the previous
implementation and Mesos' default
behaviour).

How was this patch tested?

I ran a sample application including this change on a Mesos cluster and verified the correct behaviour for both, with and without, force pulling the executor image. As expected the image is being force pulled if the flag is set.

philipphoffmann · 2016-05-11T14:40:55Z

Please note that I had to upgrade the Mesos library version to implement this feature! Unfortunately its only available in Mesos 0.22 and above.

mgummelt · 2016-05-12T17:37:53Z

docs/running-on-mesos.md

Docker image support, generally, will still work in 0.20.1. It's just the forcePullImage feature that will require 0.22.2.

True, rewrote that part.

philipphoffmann · 2016-05-27T18:14:12Z

I rebased again, against master. Anything else I can do here to get this merged?

tnachen · 2016-06-02T16:58:51Z

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

We don't really spell out the parameter name in other places in this call. I recommend just putting the value here.

tnachen · 2016-06-02T17:00:40Z

Other than the style nit, I think it LGTM. Once you fixed it we need to ask a Spark committer to review it.

tnachen · 2016-06-02T17:13:55Z

ok to test

SparkQA · 2016-06-02T17:19:34Z

Test build #59858 has finished for PR 13051 at commit 1b295e8.

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

philipphoffmann · 2016-06-02T18:00:41Z

@tnachen I addressed your comment and also fixed the scala style error from the build.

SparkQA · 2016-06-02T20:11:28Z

Test build #59864 has finished for PR 13051 at commit 80d1e3e.

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

tnachen · 2016-06-02T21:25:46Z

@andrewor14 PTAL, this PR LGTM

philipphoffmann · 2016-07-04T09:50:42Z

rebase to master

SparkQA · 2016-07-04T13:04:18Z

Test build #61719 has finished for PR 13051 at commit 7768589.

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

mgummelt · 2016-07-05T16:59:31Z

Can you add this to the CoarseMesosSchedulerBackend as well? This solution adds the force pull feature to drivers, but not executors for spark jobs.

mgummelt · 2016-07-05T17:02:06Z

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

I don't like the redundancy in having both methods. Can we simplify here?

SparkQA · 2016-07-14T14:38:22Z

Test build #62317 has finished for PR 13051 at commit c2d4b7f.

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

mgummelt · 2016-07-15T21:51:16Z

...main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala

This seems like a gratuitous conversion. I'd rather retain the semantics of SparkConf, and just pass that in, unless there's a really good reason not to.

I agree, I provided a better approach. Note that this method is also called from MesosClusterScheduler which maintains the driver settings as a raw Map[String, String] so passing the SparkConf here is not an option imho.

Mesos agents by default will not pull docker images which are cached locally already. In order to run Spark executors from mutable tags like `:latest` this commit introduces a Spark setting (`spark.mesos.executor.docker.forcePullImage`). Setting this flag to true will tell the Mesos agent to force pull the docker image (default is `false` which is consistent with the previous implementation and Mesos' default behaviour).

philipphoffmann · 2016-07-18T10:57:55Z

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

Working on this, I figured that the network parameter is used nowhere. Just wanted to point this out, not sure if there is WIP for adding this feature.

Yea, I'm not sure what happened here. Thanks for pointing it out.

SparkQA · 2016-07-18T13:44:34Z

Test build #62465 has finished for PR 13051 at commit 36b3258.

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

mgummelt · 2016-07-18T17:48:10Z

@andrewor14 This LGTM. Can you take a look and merge?

tnachen · 2016-07-25T02:43:46Z

@srowen Or if you could help :)

srowen · 2016-07-25T19:15:42Z

Merged to master

JoshRosen · 2016-07-25T19:42:56Z

This patch just broke the build:

[error] /home/jenkins/workspace/spark-master-compile-maven-hadoop-2.2/core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala:268: not enough arguments for method verifyTaskLaunched: (driver: org.apache.mesos.SchedulerDriver, offerId: String)List[org.apache.mesos.Protos.TaskInfo].
[error] Unspecified value parameter offerId.
[error]     val launchedTasks = verifyTaskLaunched("o1").asScala
[error]                                           ^
[error] /home/jenkins/workspace/spark-master-compile-maven-hadoop-2.2/core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala:306: not enough arguments for method verifyTaskLaunched: (driver: org.apache.mesos.SchedulerDriver, offerId: String)List[org.apache.mesos.Protos.TaskInfo].
[error] Unspecified value parameter offerId.
[error]     val launchedTasks = verifyTaskLaunched("o1").asScala
[error]                                           ^
[warn] one warning found
[error] two errors found
[error] Compile failed at Jul 25, 2016 12:34:51 PM [37.534s]

Going to revert now.

srowen · 2016-07-25T19:49:11Z

Shoot, yeah PR builder tests passed, but that was 7 days ago. Sorry @philipphoffmann can you try this one again after figuring out whatever caused this?

philipphoffmann · 2016-07-25T20:19:01Z

@srowen fixed the above issue and sent another request here: #14348

mgummelt reviewed May 12, 2016
View reviewed changes

philipphoffmann force-pushed the force-pull-image branch from de28d19 to 5b072ba Compare May 12, 2016 19:55

philipphoffmann force-pushed the force-pull-image branch from 5b072ba to 1b295e8 Compare May 27, 2016 18:13

tnachen reviewed Jun 2, 2016
View reviewed changes

philipphoffmann force-pushed the force-pull-image branch from 1b295e8 to 80d1e3e Compare June 2, 2016 17:59

philipphoffmann force-pushed the force-pull-image branch from 80d1e3e to 7768589 Compare July 4, 2016 09:50

mgummelt reviewed Jul 5, 2016
View reviewed changes

philipphoffmann force-pushed the force-pull-image branch from 7768589 to c2d4b7f Compare July 14, 2016 12:39

mgummelt reviewed Jul 15, 2016
View reviewed changes

philipphoffmann force-pushed the force-pull-image branch from c2d4b7f to 36b3258 Compare July 18, 2016 10:50

philipphoffmann reviewed Jul 18, 2016
View reviewed changes

asfgit closed this in 978cd5f Jul 25, 2016

srowen mentioned this pull request Jul 25, 2016

[SPARK-16637] Unified containerizer #14275

Closed

[SPARK-15271] [MESOS] Allow force pulling executor docker images #13051

[SPARK-15271] [MESOS] Allow force pulling executor docker images #13051

Uh oh!

Conversation

philipphoffmann commented May 11, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

philipphoffmann commented May 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philipphoffmann commented May 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnachen commented Jun 2, 2016

Uh oh!

tnachen commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

philipphoffmann commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

tnachen commented Jun 2, 2016

Uh oh!

philipphoffmann commented Jul 4, 2016

Uh oh!

SparkQA commented Jul 4, 2016

Uh oh!

mgummelt commented Jul 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 14, 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!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 18, 2016

Uh oh!

mgummelt commented Jul 18, 2016

Uh oh!

tnachen commented Jul 25, 2016

Uh oh!

srowen commented Jul 25, 2016

Uh oh!

JoshRosen commented Jul 25, 2016

Uh oh!

srowen commented Jul 25, 2016

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

philipphoffmann commented Jul 25, 2016 •

edited

Loading