[SPARK-20078] [MESOS] Mesos executor configurability for task name and labels #17404
Conversation
|
I agree that "Task X" is a bad naming scheme, but I'm not convinced we need to make it configurable. What if we just changed it to "[driver-name] X", so that tasks were distinguishable between drivers? Also, what problem are you trying to solve by making labels configurable? Even if we decide that's a good idea, please make a new JIRA and PR for that. It's a different task. |
|
Agreed, I made it configurable just to avoid changing what the default was, but we were just configuring it with the app name anyway. Are you thinking of using We run a lot of our streaming jobs in client mode, and as we move to having more and more tenants in our cluster, we'd like to be able to label which job belongs to who, and we can use those labels to tag the logs coming from each job for centralized logging. We're trying to link the driver logs with the executor logs. When we query the mesos API for all the running tasks/frameworks there's no easy way to link drivers to executors. Looking at the framework_id doesn't work since the dirver's framework_id is the parent (marathon in this case). And the executors have no information through that API that we can use to link to the driver. Should I make a new JIRA/PR and discuss it there? Or should we discuss it here before I open those up? |
|
RE: task names. Great, so let's drop the configuration and just use spark.app.name RE: labels. I don't understand why you can't link tasks and drivers. The driver is a framework. It's framework_id is not marathon's framework_id. It has its own framework_id, which is in fact configurable via spark.mesos.driver.frameworkId. |
|
Yeah the driver does have its own framework id, which we can find in from frameworks, but when we look at all the running tasks, each task's framework id is it's parents framework id. So the driver task shows up with marathon's framework id, and the executors show up with the framework id of the driver. So when we get the driver logs and look at the ExexutorInfo, it doesn't have the newly registered framework id. Didn't realize it was configurable though. Is there a reason not to add label support ? |
|
Oh, I see. OK, I think labels is a fine feature. Please submit in a different JIRA/PR. |
| val launchedTasks = verifyTaskLaunched(driver, "o1") | ||
|
|
||
| // Add " 0" to the taskName to match the executor number that is appended | ||
| assert(launchedTasks.head.getName == s"${sc.appName} 0") |
There was a problem hiding this comment.
Let's use the actual literal string here, rather than a parameterized string, so we can directly assert the task name we want.
| val launchedTasks = verifyTaskLaunched(driver, "o1") | ||
|
|
||
| // Add " 0" to the taskName to match the executor number that is appended | ||
| assert(launchedTasks.head.getName == "test-mesos-dynamic-alloc 0") |
There was a problem hiding this comment.
Thanks, as a side note, we need to update this task name, since it's not general to all tests here. But we can do that later.
|
Thanks for the contribution! @srowen LGTM. Please merge. |
|
Test build #3609 has finished for PR 17404 at commit
|
|
Merged to master |
## 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 apache#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/test` Please review http://spark.apache.org/contributing.html before opening a pull request. Author: Kalvin Chau <kalvin.chau@viasat.com> Closes apache#17413 from kalvinnchau/mesos-labels.
What changes were proposed in this pull request?
Adding configurable mesos executor names and labels using
spark.mesos.task.nameandspark.mesos.task.labels.Labels were defined as
k1:v1,k2:v2.@mgummelt
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.