[SPARK-21000][MESOS] Add Mesos labels support to the Spark Dispatcher #18220
Conversation
|
Test build #77780 has finished for PR 18220 at commit
|
|
Test build #77781 has finished for PR 18220 at commit
|
|
Test build #77782 has finished for PR 18220 at commit
|
|
Test build #77785 has finished for PR 18220 at commit
|
|
Test build #77786 has finished for PR 18220 at commit
|
| } | ||
| } | ||
|
|
||
| while(i < labelsStr.length) { |
There was a problem hiding this comment.
Ideally, this would be more functional. I tried to model it with map/fold, but I'm not smart enough. If someone cares, I can try to rewrite it to be recursive, at least.
| @@ -469,6 +470,15 @@ See the [configuration page](configuration.html) for information on Spark config | |||
| </td> | |||
| </tr> | |||
| <tr> | |||
| <td><code>spark.mesos.driver.labels</code></td> | |||
There was a problem hiding this comment.
This naming differs a bit from the YARN label property, but I suppose it's supporting an expression. And we have .task.labels already. OK.
docs/running-on-mesos.md
Outdated
| <td><code>spark.mesos.driver.labels</code></td> | ||
| <td><code>(none)</code></td> | ||
| <td> | ||
| Mesos labels to add to the driver. See spark.mesos.task.labels |
There was a problem hiding this comment.
Nit: format props and code with <code>
| value = Some(currStr) | ||
| if (key.isEmpty) { | ||
| throw new SparkException(s"Error while parsing label string: ${labelsStr}. " + | ||
| s"Empty label key.") |
There was a problem hiding this comment.
Nit, don't need interpolation but whatever
| } | ||
| } | ||
|
|
||
| while(i < labelsStr.length) { |
There was a problem hiding this comment.
this code was removed, so this is fixed.
|
|
||
| // 0 -> parsing key | ||
| // 1 -> parsing value | ||
| var state = 0 |
There was a problem hiding this comment.
This all looks excessively complex. Can't you do this with a regex in a few lines?
There was a problem hiding this comment.
I simplified it, but I can't do a simple regex splitting, because I have to condition the match on characters (the escape sequence), that shouldn't actually be considered part of the matched string. So I just wrote a custom splitUnescaped method to implement what I need.
|
Test build #77816 has finished for PR 18220 at commit
|
|
Test build #77817 has finished for PR 18220 at commit
|
|
Test build #77818 has finished for PR 18220 at commit
|
|
@srowen All comments addressed. Tests are passing. |
| def mesosLabels(labelsStr: String): Protos.Labels.Builder = { | ||
|
|
||
| // Return str split around unescaped occurrences of c. | ||
| def splitUnescaped(str: String, c: Char): Seq[String] = { |
There was a problem hiding this comment.
I still think you can do this much more directly with regexes, even with escapes. It looks a little tricky but a negative lookbehind does the trick. For example:
scala> """key:value,key2:a\:b,key3:a\,b""".split("""(?<!\\),""")
res3: Array[String] = Array(key:value, key2:a\:b, key3:a\,b)
There was a problem hiding this comment.
That's exactly what I was looking for! Thanks! Fixed.
|
Test build #77851 has finished for PR 18220 at commit
|
| throw new SparkException(s"Malformed label: ${labelStr}") | ||
| } | ||
|
|
||
| val cleanedParts = parts |
There was a problem hiding this comment.
You could do this too, though I don't think it matters enough to bother:
val Array(key, value) = parts.map(_.replaceAll("""\\(.)""", "$1"))
|
Test build #77852 has finished for PR 18220 at commit
|
|
Test build #77855 has finished for PR 18220 at commit
|
|
Merged to master |
## What changes were proposed in this pull request? Add Mesos labels support to the Spark Dispatcher ## How was this patch tested? unit tests Author: Michael Gummelt <[email protected]> Closes apache#18220 from mgummelt/SPARK-21000-dispatcher-labels.
## What changes were proposed in this pull request? Add Mesos labels support to the Spark Dispatcher ## How was this patch tested? unit tests Author: Michael Gummelt <[email protected]> Closes apache#18220 from mgummelt/SPARK-21000-dispatcher-labels.
Add Mesos labels support to the Spark Dispatcher unit tests Author: Michael Gummelt <[email protected]> Closes apache#18220 from mgummelt/SPARK-21000-dispatcher-labels.
What changes were proposed in this pull request?
Add Mesos labels support to the Spark Dispatcher
How was this patch tested?
unit tests