[SPARK-6350][Mesos] Fine-grained mode scheduler respects mesosExecutor.cores #8653
Conversation
|
I'm working on another fix for SPARK-7874, and will remove some of the duplication in tests, but I wanted to keep this PR as simple as possible (in line with the trivial fix). |
|
Test build #42132 has finished for PR 8653 at commit
|
|
/cc @skyluc, @nraychaudhuri |
| // uri is null. | ||
| val (executorInfo, _) = mesosSchedulerBackend.createExecutorInfo(resources, "test-id") | ||
| val executorResources = executorInfo.getResourcesList | ||
| val cpus = executorResources.asScala.find(_.getName == "cpus").get.getScalar.getValue |
There was a problem hiding this comment.
Nit: I would prefer we use the same matching logic (equals) just to be consistent.
There was a problem hiding this comment.
You mean in the assertion below? Not sure what you mean (I think === offers a more detailed error message, but I am ok changing it to == if that's what you mean).
There was a problem hiding this comment.
I just mean _.getName.equals("cpus") :) Not a big deal, just like to be consistent
There was a problem hiding this comment.
Oh, I see. I can do that. Do you know why equals is preferred? == is safe w.r.t. to nulls, so we assume getName (and all other getters, like getContainerPath) never return null?
There was a problem hiding this comment.
I don't think it's really preferred, it's just how it's written in the first place. I suggest if you think it makes sense either keep it for now or change it everywhere else.
|
Good catch! Indeed a regression... this PR LGTM @andrewor14 |
spark.mesos.mesosExecutor.cores when launching Mesos executors (regression)
dragos
force-pushed
the
issue/mesos/fine-grained-maxExecutorCores
branch
from
5cd9343 to
03e8d0a
Compare
|
Test build #42197 has finished for PR 8653 at commit
|
|
Any committer wants to have a final look? It's a one-liner, but #8671 depends on this (currently I have an ignored test in there, pending this fix). /cc @andrewor14 (@srowen too, in case you have bandwidth..) |
|
LGTM merging into master 1.5 thanks @dragos. |
…or.cores This is a regression introduced in #4960, this commit fixes it and adds a test. tnachen andrewor14 please review, this should be an easy one. Author: Iulian Dragos <[email protected]> Closes #8653 from dragos/issue/mesos/fine-grained-maxExecutorCores. (cherry picked from commit f0562e8) Signed-off-by: Andrew Or <[email protected]>
This is a regression introduced in #4960, this commit fixes it and adds a test.
@tnachen @andrewor14 please review, this should be an easy one.