Uh oh!
There was an error while loading. Please reload this page.
[SPARK-6707] [CORE][MESOS]: Mesos Scheduler should allow the user to specify constraints based on slave attributes - #5563
[SPARK-6707] [CORE][MESOS]: Mesos Scheduler should allow the user to specify constraints based on slave attributes#5563ankurcha wants to merge 24 commits into
Conversation
AmplabJenkins
commented
Apr 17, 2015
Can one of the admins verify this patch? |
There was a problem hiding this comment.
I don't ever see usage of a package object in Spark, not sure we'd like to set a precedent here.
@andrewor14 is more familiar with the style I'll let him comment on this, but I'll recommend not doing this.
There was a problem hiding this comment.
we do have package objects :)
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/package.scala
There was a problem hiding this comment.
:) i see, do you recommend using it like this? every package.scala seems to be just defining an object
There was a problem hiding this comment.
Yeah that's a good point. I think the convention elsewhere is that we define a XUtils object and do XUtils.methodName() for common methods (see Utils, JettyUtils, AkkaUtils etc.). It might make more sense to do the same here.
There was a problem hiding this comment.
Added MesosUtils. I am not a big fan of this name so if you have a better one, please let me know.
tnachen
commented
Apr 21, 2015
Btw why only apply constraints on fine grain mode? Why not coarse grain? |
andrewor14
commented
Apr 21, 2015
Jenkins, this is ok to test, but we will need to rebase this to master to resolve the merge conflicts. |
SparkQA
commented
Apr 21, 2015
Test build #30693 has started for PR 5563 at commit |
ankurcha
commented
Apr 21, 2015
Thanks all, I'll make the changes and rebase the pull request. This is my first foray into the world of any "real" scala so really appreciate the feedback. |
AmplabJenkins
commented
Apr 21, 2015
Test FAILed. |
ankurcha
commented
Apr 21, 2015
@tnachen - I added support to the coarse scheduler too. I had missed that one. |
AmplabJenkins
commented
Apr 21, 2015
Test FAILed. |
SparkQA
commented
Apr 21, 2015
Test build #30703 has started for PR 5563 at commit |
SparkQA
commented
Apr 21, 2015
Test build #30703 has finished for PR 5563 at commit
|
AmplabJenkins
commented
Apr 21, 2015
Test FAILed. |
AmplabJenkins
commented
Apr 21, 2015
Test FAILed. |
SparkQA
commented
Apr 21, 2015
Test build #30693 has finished for PR 5563 at commit
|
AmplabJenkins
commented
Apr 21, 2015
Test PASSed. |
SparkQA
commented
Apr 24, 2015
Test build #30944 has started for PR 5563 at commit |
SparkQA
commented
Apr 24, 2015
Test build #30944 has finished for PR 5563 at commit
|
AmplabJenkins
commented
Apr 24, 2015
Test FAILed. |
SparkQA
commented
Apr 24, 2015
Test build #30945 has started for PR 5563 at commit |
AmplabJenkins
commented
Apr 27, 2015
Can one of the admins verify this patch? |
There was a problem hiding this comment.
scala imports needs to be after java imports according to spark style guide, please move in between java and org.apache imports.
There was a problem hiding this comment.
thanks for rewriting this!! The old code is unbelievably dense.
andrewor14
commented
Jul 2, 2015
@ankurcha thanks for spending the time on this feature. This patch is very well documented and refactors the mesos integration code in a way that makes sense. LGTM from the Spark side. Unfortunately I'm not as well-versed in Mesos as @tnachen@dragos are. Any other comments from your side? Have we done more testing after the latest changes? Should we add a TODO comment somewhere for more complex operators? |
ankurcha
commented
Jul 2, 2015
@andrewor14 - I have addressed your comments in d83801c |
dragos
commented
Jul 3, 2015
To me this looks good, I just didn't have the time to run it on our Mesos cluster again. I'll try to do so ASAP, but in the meantime maybe @tnachen or @deanwampler give it a go. |
tnachen
commented
Jul 3, 2015
One more thing after looking at the mesos code more closely (haven't really looked and touched attributes at all while working on mesos), we basically support ranges, scalar or text. Set is not supported for attributes, not sure why but attributes hasn't been touched since 2012. |
andrewor14
commented
Jul 4, 2015
retest this please |
AmplabJenkins
commented
Jul 4, 2015
Merged build triggered. |
AmplabJenkins
commented
Jul 4, 2015
Merged build started. |
SparkQA
commented
Jul 4, 2015
Test build #36518 has started for PR 5563 at commit |
SparkQA
commented
Jul 4, 2015
Test build #36518 has finished for PR 5563 at commit
|
AmplabJenkins
commented
Jul 4, 2015
Merged build finished. Test PASSed. |
ankurcha
commented
Jul 6, 2015
@tnachen - Yes i did test this on my cluster (3x master + 3x slaves ) for the calculate Pi example mentioned above. |
dragos
commented
Jul 6, 2015
I tried this on a 2-node Mesos cluster. I confirm that I could use numeric values, and it worked as expected. and my two slaves had an attribute |
dragos
commented
Jul 6, 2015
This looks good to me! |
nollbit
commented
Jul 6, 2015
We've been running this patch in production for a few weeks now. Apart from the (now fixed) bug where it would not properly decline unused offers, we've not had any issues. |
andrewor14
commented
Jul 6, 2015
LGTM2. I am merging this into master. Sorry to all other mesos patches that this one conflicts with! |
tnachen
commented
Jul 6, 2015
LGTM as well |
andrewor14
commented
Jul 6, 2015
(There's a problem with the infra that prevents me from merging this. I'll try again in a few hours) |
Currently, the mesos scheduler only looks at the 'cpu' and 'mem' resources when trying to determine the usablility of a resource offer from a mesos slave node. It may be preferable for the user to be able to ensure that the spark jobs are only started on a certain set of nodes (based on attributes).
For example, If the user sets a property, let's say
spark.mesos.constraintsis set totachyon=true;us-east-1=false, then the resource offers will be checked to see if they meet both these constraints and only then will be accepted to start new executors.