[SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. #1462

li-zhihui · 2014-07-17T09:31:58Z

In SPARK-1946(PR #900), configuration spark.scheduler.minRegisteredExecutorsRatio was introduced(and it was renamed spark.scheduler.minRegisteredResourcesRatio in PR #1525), but it only support Standalone and Yarn mode.
This PR try to introduce the configuration to Mesos mode.

In Mesos coarse-grained mode, the configuration is work well.
In Mesos fine-grained mode, the configuration is ignored because executors are dynamic, and SchedulerBackend will print a warning log if the configuration is set.

AmplabJenkins · 2014-07-17T09:36:39Z

Can one of the admins verify this patch?

li-zhihui · 2014-07-17T09:40:43Z

@tgravescs

tgravescs · 2014-07-18T14:03:58Z

Did you test it on a cluster? I unfortunately don't have access to one and am not an expert on mesos.

Is there a race condition between when the scheduler backend increments totalExpectedExecutors and when we actually do the check to see if we have enough? Meaning in this case we increment it as they come in as resource offers, we start the executors, it registers, then we do the check, so its possible once we get 1 in that totalExpectedExecutors =1 * minRegisteredRatio (say 100) == executorActor.size() (1) even though we really expect say 10 to come in?

I think the same thing actually applies in standalone mode too but I missed it in previous pr.

li-zhihui · 2014-07-21T05:10:29Z

@tgravescs

I tested it on a cluster with mesos-0.18.1(fine-grained and coarse-grained), it work well.

I think you are right. In fact, user don't have any idea about expected executors in mesos mode (and standalone mode), they only expect CPU cores(spark.cores.max). So we need check total registered executors' cores and spark.cores.max to judge whether SchedulerBackend is ready, and modify spark.scheduler.minRegisteredExecutorsRatio to spark.scheduler.minRegisteredResourcesRatio.
How do you think about it?

tgravescs · 2014-07-21T14:56:56Z

cc @kayousterhout as I think she is more familiar with standalone mode and scheduler details.

tgravescs · 2014-07-21T18:16:46Z

If we change the name of the config you'll need to upmerge as #634 set some defaults on the yarn side.

kayousterhout · 2014-07-21T20:35:30Z

@tgravescs I actually mentioned this race condition in the previous PR: #900 (comment) . In the future we should try to be more careful about merging things that have un-replied to comments (I'm about to send an email to the dev list about this).

@li-zhihui if someone points out a problem in a pull request you submit, the expectation is that it will be fixed when you reply to the comment. Can you please submit a new pull request that fixes the race condition with standalone mode, before we proceed with adding this functionality to Mesos mode?

tgravescs · 2014-07-21T21:35:14Z

Sorry @kayousterhout I totally missed it, I didn't read close enough and thought it had been addressed.

kayousterhout · 2014-07-21T21:37:22Z

No worries I should have just made a top-level comment -- the code-level
comments are easy to miss once they get compressed because the code is out
of date.

On Mon, Jul 21, 2014 at 2:35 PM, Tom Graves [email protected]
wrote:

Sorry @kayousterhout https://github.com/kayousterhout I totally missed
it, I didn't read close enough and thought it had been addressed.

—
Reply to this email directly or view it on GitHub
#1462 (comment).

li-zhihui · 2014-07-22T02:15:43Z

Sorry @tgravescs @kayousterhout I am not aware of the issue's seriousness at that time. thanks @kayousterhout for your coach.

tnachen · 2014-07-28T21:30:30Z

Are we trying to figure out the top level issue of the race before we get this in?

li-zhihui · 2014-07-29T01:08:18Z

@tnachen I add a new PR to try to fix the issue, #1525

pwendell · 2014-08-04T03:24:14Z

I'd like to see a version of this patch that simply removes support for this in standalone mode - at least so we can do the comparison. @li-zhihui, why can't a user just write this in their own code? It seems really simple...

li-zhihui · 2014-08-04T05:41:09Z

@pwendell removing support for this in standalone mode is just keeping totalExpectedExecutors zero.
li-zhihui@fa5af15

I think it just make user use spark more easily. (And sometimes user isn't aware of the problem unless we show them by docs or conf). Anyway, I think you are the authority on how to make the tradeoff.

@pwendell

Current code waits until some minimum fraction of expected executors have registered before beginning scheduling. The current code in standalone mode suffers from a race condition (SPARK-2635). This race condition could be fixed, but this functionality is easily achieved by the user (they can use the storage status to determine how many executors are up, as described by @pwendell in apache#1462) so adding the extra complexity to the scheduler code is not worthwile.

kayousterhout · 2014-08-04T07:06:18Z

@pwendell I created #1762 for your judgment of what the right thing to do here is!

pwendell · 2014-08-04T07:33:38Z

Okay let me run it by some more people tomorrow and figure it out.

li-zhihui · 2014-08-25T06:05:49Z

Rollback old commits, add a new commit base on latest code.

@pwendell @tgravescs @kayousterhout @tnachen

SparkQA · 2014-09-05T23:45:10Z

Can one of the admins verify this patch?

nchammas · 2015-02-12T03:20:57Z

This PR has gone stale. Do we want to update it?

li-zhihui · 2015-02-12T04:14:32Z

@pwendell Do we need the feature in mesos mode? I am pleasure to update it.

andrewor14 · 2015-02-19T20:35:08Z

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

@@ -62,6 +62,11 @@ private[spark] class MesosSchedulerBackend(

  var classLoader: ClassLoader = null

+  if (!sc.getConf.getOption("spark.scheduler.minRegisteredResourcesRatio").isEmpty) {


sc.conf.contains(...)

Done, thanks.

andrewor14 · 2015-02-19T20:37:08Z

@pwendell @kayousterhout what is the verdict of this? Should we just remove the ratio altogether? What about backward compatibility?

…Mesos mode.

li-zhihui · 2015-02-26T05:11:31Z

Add some new commits to fix code conflict and some issues.

…ourcesRatio on docs. The configuration is not supported in mesos mode now. See #1462 Author: Li Zhihui <[email protected]> Closes #4781 from li-zhihui/fixdocconf and squashes the following commits: 63e7a44 [Li Zhihui] Modify default value description for spark.scheduler.minRegisteredResourcesRatio on docs. (cherry picked from commit 10094a5) Signed-off-by: Andrew Or <[email protected]>

…ourcesRatio on docs. The configuration is not supported in mesos mode now. See #1462 Author: Li Zhihui <[email protected]> Closes #4781 from li-zhihui/fixdocconf and squashes the following commits: 63e7a44 [Li Zhihui] Modify default value description for spark.scheduler.minRegisteredResourcesRatio on docs.

andrewor14 · 2015-02-26T22:33:49Z

@li-zhihui I didn't realize the spark.scheduler.minRegisteredResourcesRatio defaulted to 0 in standalone mode. Given that I think it's safe to remove support for it, since it will only affect people who explicitly set this. For 1.4 we can deprecate it in standalone mode and for 1.5+ we can remove it.

Given that would you mind closing this?

li-zhihui mentioned this pull request Aug 2, 2014

[SPARK-2635] Fix race condition at SchedulerBackend.isReady in standalone mode #1525

Closed

kayousterhout mentioned this pull request Aug 4, 2014

[POC] Remove support for waiting for executors in standalone mode. #1762

Closed

li-zhihui force-pushed the re4mesos branch from 2f4f101 to 1e81fb4 Compare August 25, 2014 03:04

li-zhihui changed the title ~~[SPARK-2555] Support configuration spark.scheduler.minRegisteredExecutorsRatio in Mes...~~ [SPARK-2555] Support configuration spark.scheduler.minRegisteredExecutorsRatio in Mesos mode. Aug 25, 2014

li-zhihui changed the title ~~[SPARK-2555] Support configuration spark.scheduler.minRegisteredExecutorsRatio in Mesos mode.~~ [SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. Aug 25, 2014

andrewor14 reviewed Feb 19, 2015
View reviewed changes

Support configuration spark.scheduler.minRegisteredResourcesRatio in …

4afbaf0

…Mesos mode.

li-zhihui added 2 commits February 26, 2015 12:47

Fix some code and doc issues

17213f5

Fix docs issue

ebdb664

li-zhihui force-pushed the re4mesos branch from 1e81fb4 to 4afbaf0 Compare February 26, 2015 04:56

li-zhihui mentioned this pull request Feb 26, 2015

Modify default value description for spark.scheduler.minRegisteredResourcesRatio on docs. #4781

Closed

li-zhihui closed this Feb 27, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. #1462

[SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. #1462

li-zhihui commented Jul 17, 2014

AmplabJenkins commented Jul 17, 2014

li-zhihui commented Jul 17, 2014

tgravescs commented Jul 18, 2014

li-zhihui commented Jul 21, 2014

tgravescs commented Jul 21, 2014

tgravescs commented Jul 21, 2014

kayousterhout commented Jul 21, 2014

tgravescs commented Jul 21, 2014

kayousterhout commented Jul 21, 2014

li-zhihui commented Jul 22, 2014

tnachen commented Jul 28, 2014

li-zhihui commented Jul 29, 2014

pwendell commented Aug 4, 2014

li-zhihui commented Aug 4, 2014

kayousterhout commented Aug 4, 2014

pwendell commented Aug 4, 2014

li-zhihui commented Aug 25, 2014

SparkQA commented Sep 5, 2014

nchammas commented Feb 12, 2015

li-zhihui commented Feb 12, 2015

andrewor14 Feb 19, 2015

li-zhihui Feb 26, 2015

andrewor14 commented Feb 19, 2015

li-zhihui commented Feb 26, 2015

andrewor14 commented Feb 26, 2015

		@@ -62,6 +62,11 @@ private[spark] class MesosSchedulerBackend(

		var classLoader: ClassLoader = null

		if (!sc.getConf.getOption("spark.scheduler.minRegisteredResourcesRatio").isEmpty) {

[SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. #1462

[SPARK-2555] Support configuration spark.scheduler.minRegisteredResourcesRatio in Mesos mode. #1462

Conversation

li-zhihui commented Jul 17, 2014

AmplabJenkins commented Jul 17, 2014

li-zhihui commented Jul 17, 2014

tgravescs commented Jul 18, 2014

li-zhihui commented Jul 21, 2014

tgravescs commented Jul 21, 2014

tgravescs commented Jul 21, 2014

kayousterhout commented Jul 21, 2014

tgravescs commented Jul 21, 2014

kayousterhout commented Jul 21, 2014

li-zhihui commented Jul 22, 2014

tnachen commented Jul 28, 2014

li-zhihui commented Jul 29, 2014

pwendell commented Aug 4, 2014

li-zhihui commented Aug 4, 2014

kayousterhout commented Aug 4, 2014

pwendell commented Aug 4, 2014

li-zhihui commented Aug 25, 2014

SparkQA commented Sep 5, 2014

nchammas commented Feb 12, 2015

li-zhihui commented Feb 12, 2015

andrewor14 Feb 19, 2015

Choose a reason for hiding this comment

li-zhihui Feb 26, 2015

Choose a reason for hiding this comment

andrewor14 commented Feb 19, 2015

li-zhihui commented Feb 26, 2015

andrewor14 commented Feb 26, 2015