[SPARK-20085] [MESOS] Configurable mesos labels for executors#17413
[SPARK-20085] [MESOS] Configurable mesos labels for executors#17413kalvinnchau wants to merge 3 commits intoapache:masterfrom
Conversation
|
Test build #3610 has finished for PR 17413 at commit
|
kalvinnchau
force-pushed
the
mesos-labels
branch
from
c57afed to
97a0d50
Compare
|
@mgummelt bumping this, can we get this looked at please? |
| <tr> | ||
| <td><code>spark.mesos.task.labels</code></td> | ||
| <td>(none)</td> | ||
| <td> |
|
|
||
| val labelsBuilder = taskBuilder.getLabelsBuilder | ||
|
|
||
| taskLabels.split(",").foreach(label => { |
There was a problem hiding this comment.
Please create a separate method for this.
|
LGTM. @srowen Can we please get a merge? Thanks. |
srowen
left a comment
srowen
left a comment
There was a problem hiding this comment.
Looks OK, just some minor suggestions on the code itself
| } | ||
|
|
||
| private def buildMesosLabels() : List[Label] = { | ||
| taskLabels.split(",").flatMap(label => { |
There was a problem hiding this comment.
Nit: the braces are redundant here? or else use .flatMap { label => syntax
There was a problem hiding this comment.
removed the redundant braces here
| tasks.toMap | ||
| } | ||
|
|
||
| private def buildMesosLabels() : List[Label] = { |
There was a problem hiding this comment.
Nit: no space before colon I think in our code style. Does it need to be a List for ScalaConverters to convert to a Java List or would it work with whatever the flatMap returns already (Seq?) No big deal.
There was a problem hiding this comment.
Removed the space
The flatMap returns an Array, which doesn't work the with ScalaConverters, and the addAlllabels didn't take that Array.
|
|
||
| private val maxGpus = conf.getInt("spark.mesos.gpus.max", 0) | ||
|
|
||
| private val taskLabels = conf.get("spark.mesos.task.labels", "").toString |
There was a problem hiding this comment.
Is toString redundant here or I miss something?
There was a problem hiding this comment.
whoops, that is redundant, i'll drop that toString
|
Test build #3641 has finished for PR 17413 at commit
|
|
Merged to master |
4 participants
What changes were proposed in this pull request?
Add spark.mesos.task.labels configuration option to add mesos key:value labels to the executor.
"k1:v1,k2:v2" as the format, colons separating key-value and commas to list out more than one.
Discussion of labels with @mgummelt at #17404
How was this patch tested?
Added unit tests to verify labels were added correctly, with incorrect labels being ignored and added a test to test the name of the executor.
Tested with:
./build/sbt -Pmesos mesos/testPlease review http://spark.apache.org/contributing.html before opening a pull request.