[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/8872 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-187324256 No worries, thanks for addressing them. I'm merging this into master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186534096 @andrewor14 Sorry for the mess up, I kept thinking the code was ready just needed to rebase and address comments. The rebase and comments did cause some style problems in the end, will be more careful next time. I took a pass again and I don't see anything any more. PTAL when you can. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r53546468
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -442,9 +443,12 @@ private[spark] class MesosClusterScheduler(
options
}
- private class ResourceOffer(val offer: Offer, var cpu: Double, var mem:
Double) {
+ private class ResourceOffer(
+ val offerId: OfferID,
--- End diff --
Sounds good, will remember this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186486243 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186486247 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51582/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186485951 **[Test build #51582 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51582/consoleFull)** for PR 8872 at commit [`ebadaf3`](https://github.com/apache/spark/commit/ebadaf3c55e839a00a8cf534e7b2fd2c8e988475). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186459730 **[Test build #51582 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51582/consoleFull)** for PR 8872 at commit [`ebadaf3`](https://github.com/apache/spark/commit/ebadaf3c55e839a00a8cf534e7b2fd2c8e988475). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186454509 retest please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186454538 retest this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186453924 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186453926 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51581/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-186443529 @tnachen I don't think this patch is quite ready to be merged yet; the code quality can still improve in a few places I pointed out in my comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r53531547
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -442,9 +443,12 @@ private[spark] class MesosClusterScheduler(
options
}
- private class ResourceOffer(val offer: Offer, var cpu: Double, var mem:
Double) {
+ private class ResourceOffer(
+ val offerId: OfferID,
--- End diff --
in the future I will keep minor refactors like this separate from this
patch. It's distracting to the reviewer and is not needed to fix SPARK-10749.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r53531469
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -452,38 +456,42 @@ private[spark] class MesosClusterScheduler(
* This method takes all the possible candidates and attempt to schedule
them with Mesos offers.
* Every time a new task is scheduled, the afterLaunchCallback is called
to perform post scheduled
* logic on each task.
+ *
+ * @return tasks Remaining resources after scheduling tasks
*/
private def scheduleTasks(
candidates: Seq[MesosDriverDescription],
afterLaunchCallback: (String) => Boolean,
- currentOffers: List[ResourceOffer],
- tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]): Unit = {
+ currentOffers: JList[ResourceOffer],
--- End diff --
Ah yes I originally changed it because we changed the MesosSchedulerUtils
return type as I rebased, but realized now I don't have to. I'm fixing it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r53530854
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -452,38 +456,42 @@ private[spark] class MesosClusterScheduler(
* This method takes all the possible candidates and attempt to schedule
them with Mesos offers.
* Every time a new task is scheduled, the afterLaunchCallback is called
to perform post scheduled
* logic on each task.
+ *
+ * @return tasks Remaining resources after scheduling tasks
*/
private def scheduleTasks(
candidates: Seq[MesosDriverDescription],
afterLaunchCallback: (String) => Boolean,
- currentOffers: List[ResourceOffer],
- tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]): Unit = {
+ currentOffers: JList[ResourceOffer],
--- End diff --
as mentioned elsewhere, there's no reason to change this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r53530824
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -496,28 +504,29 @@ private[spark] class MesosClusterScheduler(
container, image, volumes = volumes, portmaps = portmaps)
taskInfo.setContainer(container.build())
}
-val queuedTasks = tasks.getOrElseUpdate(offer.offer.getId, new
ArrayBuffer[TaskInfo])
+val queuedTasks = tasks.getOrElseUpdate(offer.offerId, new
ArrayBuffer[TaskInfo])
queuedTasks += taskInfo.build()
-logTrace(s"Using offer ${offer.offer.getId.getValue} to launch
driver " +
+logTrace(s"Using offer ${offer.offerId.getValue} to launch driver
" +
submission.submissionId)
-val newState = new MesosClusterSubmissionState(submission, taskId,
offer.offer.getSlaveId,
+val newState = new MesosClusterSubmissionState(submission, taskId,
offer.slaveId,
None, new Date(), None)
launchedDrivers(submission.submissionId) = newState
launchedDriversState.persist(submission.submissionId, newState)
afterLaunchCallback(submission.submissionId)
}
}
+currentOffers
}
override def resourceOffers(driver: SchedulerDriver, offers:
JList[Offer]): Unit = {
-val currentOffers = offers.asScala.map(o =>
- new ResourceOffer(
-o, getResource(o.getResourcesList, "cpus"),
getResource(o.getResourcesList, "mem"))
-).toList
-logTrace(s"Received offers from Mesos:
\n${currentOffers.mkString("\n")}")
+logTrace(s"Received offers from Mesos:
\n${offers.asScala.mkString("\n")}")
val tasks = new mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]()
val currentTime = new Date()
+val currentOffers = offers.asScala.map {
+ o => new ResourceOffer(o.getId, o.getSlaveId, o.getResourcesList)
+}.toList.asJava
--- End diff --
I looked for the places where this is used. All you're doing is passing
this java list into `scheduleTasks`, where you internally converts it back to
scala again, and using it at the end of this method to decline offers, where
you convert it back to scala anyway. You really don't need to make this
`asJava` again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r53530197 --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala --- @@ -452,38 +456,42 @@ private[spark] class MesosClusterScheduler( * This method takes all the possible candidates and attempt to schedule them with Mesos offers. * Every time a new task is scheduled, the afterLaunchCallback is called to perform post scheduled * logic on each task. + * + * @return tasks Remaining resources after scheduling tasks --- End diff -- actually you don't read the return value anywhere. You can keep this as Unit. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r53530010 --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala --- @@ -452,38 +456,42 @@ private[spark] class MesosClusterScheduler( * This method takes all the possible candidates and attempt to schedule them with Mesos offers. * Every time a new task is scheduled, the afterLaunchCallback is called to perform post scheduled * logic on each task. + * + * @return tasks Remaining resources after scheduling tasks --- End diff -- you should proof-read your own comments. This shouldn't say `@return tasks`... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-184461120 @andrewor14 PTAL --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-184295000 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-184295002 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51312/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-184294723 **[Test build #51312 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51312/consoleFull)** for PR 8872 at commit [`8630442`](https://github.com/apache/spark/commit/863044207f4234d7668a6d2279df0f6fa9d7adbb). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-184237515 **[Test build #51312 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51312/consoleFull)** for PR 8872 at commit [`8630442`](https://github.com/apache/spark/commit/863044207f4234d7668a6d2279df0f6fa9d7adbb). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-183898284 We already have some shared logic of using multiple resources from different roles, it just wasn't plugged in when I wrote the cluster scheduler. I think now we have a refactored coarse grained scheduler, we can take a step back and refactor both of these so a scheduler can have pluggable pieces when it comes to launching tasks and placing resources, etc. For now I think this PR is still valid that we just want to plug in the existing multiple role logic into cluster scheduler. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r52843425 --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala --- @@ -525,14 +531,14 @@ private[spark] class MesosClusterScheduler( d.retryState.get.nextRetry.before(currentTime) } - scheduleTasks( + currentOffers = scheduleTasks( --- End diff -- Actually you're right we're just returning the value as-is, we can remove the assignment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r52843376 --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala --- @@ -525,14 +531,14 @@ private[spark] class MesosClusterScheduler( d.retryState.get.nextRetry.before(currentTime) } - scheduleTasks( + currentOffers = scheduleTasks( --- End diff -- Schedule tasks returns the remaining offers after using them, after looking at this I think I can refactor it so we don't have to make it immutable. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-178501728 For the record, [SPARK-10444](https://issues.apache.org/jira/browse/SPARK-10444) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r51481128 --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala --- @@ -0,0 +1,139 @@ +/* --- End diff -- what changed here? Did you move this file or something? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request: https://github.com/apache/spark/pull/8872#discussion_r51481006 --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala --- @@ -525,14 +531,14 @@ private[spark] class MesosClusterScheduler( d.retryState.get.nextRetry.before(currentTime) } - scheduleTasks( + currentOffers = scheduleTasks( --- End diff -- similarly I don't get the point of assigning this var every time you call `scheduleTasks`. It's the same thing, isn't it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r51480776
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -497,28 +505,28 @@ private[spark] class MesosClusterScheduler(
container, image, volumes = volumes, portmaps = portmaps)
taskInfo.setContainer(container.build())
}
-val queuedTasks = tasks.getOrElseUpdate(offer.offer.getId, new
ArrayBuffer[TaskInfo])
+val queuedTasks = tasks.getOrElseUpdate(offer.offerId, new
ArrayBuffer[TaskInfo])
queuedTasks += taskInfo.build()
-logTrace(s"Using offer ${offer.offer.getId.getValue} to launch
driver " +
+logTrace(s"Using offer ${offer.offerId.getValue} to launch driver
" +
submission.submissionId)
-val newState = new MesosClusterSubmissionState(submission, taskId,
offer.offer.getSlaveId,
+val newState = new MesosClusterSubmissionState(submission, taskId,
offer.slaveId,
None, new Date(), None)
launchedDrivers(submission.submissionId) = newState
launchedDriversState.persist(submission.submissionId, newState)
afterLaunchCallback(submission.submissionId)
}
}
+currentOffers
--- End diff --
@tnachen I also don't get it. If the caller called us with `currentOffers`,
they already have access to it, so what's the point in returning them here?
AFAIK we don't mutate this list in this method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r51480535
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -456,34 +460,36 @@ private[spark] class MesosClusterScheduler(
private def scheduleTasks(
candidates: Seq[MesosDriverDescription],
afterLaunchCallback: (String) => Boolean,
- currentOffers: List[ResourceOffer],
- tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]): Unit = {
+ currentOffers: JList[ResourceOffer],
+ tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]):
JList[ResourceOffer] = {
--- End diff --
what does this return? You need to document this in the javadoc
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-178195492 @tnachen echoing my comment from another PR: it seems that this feature is already supported in client mode but not in cluster mode. Is there something we can do about this divergence in behavior? Is there some duplicate code to clean up so we don't run into something like this in the future? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176599538 Looks like there are more rules to scala style now, it's finally passing! @andrewor14 PTAL --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176383856 **[Test build #50293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50293/consoleFull)** for PR 8872 at commit [`0998f41`](https://github.com/apache/spark/commit/0998f41a32097847739cfd2e5c308dfffb498927). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176381627 **[Test build #50292 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50292/consoleFull)** for PR 8872 at commit [`0998f41`](https://github.com/apache/spark/commit/0998f41a32097847739cfd2e5c308dfffb498927). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176381644 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176380674 **[Test build #50292 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50292/consoleFull)** for PR 8872 at commit [`0998f41`](https://github.com/apache/spark/commit/0998f41a32097847739cfd2e5c308dfffb498927). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176384707 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176381648 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50292/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176384712 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50293/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176384693 **[Test build #50293 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50293/consoleFull)** for PR 8872 at commit [`0998f41`](https://github.com/apache/spark/commit/0998f41a32097847739cfd2e5c308dfffb498927). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176377522 retest this please --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176563517 **[Test build #50326 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50326/consoleFull)** for PR 8872 at commit [`7a7daea`](https://github.com/apache/spark/commit/7a7daea2540b3ebd03a1d2514a9fd6c93daf93e2). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176563905 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176563907 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/50326/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-176525823 **[Test build #50326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/50326/consoleFull)** for PR 8872 at commit [`7a7daea`](https://github.com/apache/spark/commit/7a7daea2540b3ebd03a1d2514a9fd6c93daf93e2). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173841919 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173841915 **[Test build #49914 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49914/consoleFull)** for PR 8872 at commit [`93de692`](https://github.com/apache/spark/commit/93de6920faf56b516eadc2a7ba030aa003560c87). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173841921 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/49914/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173841613 **[Test build #49914 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/49914/consoleFull)** for PR 8872 at commit [`93de692`](https://github.com/apache/spark/commit/93de6920faf56b516eadc2a7ba030aa003560c87). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173931173 I tested this and can confirm it works as expected. I merged #10370 prior to testing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r50540694
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
Agree with `Nil ++`. Regarding Java <-> Scala conversions, I think this
change is a net positive: removed around 9 calls to asJava and introduced only
3 asScala. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-173932754 There's an ongoing discussions regarding one minor point. I don't have other comments, so LGTM. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-172524412 @tnachen, this PR would need to be rebased on master. At any rate, if there are fixes in cluster-mode that make it work now, they are not part of this PR yet. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-17198 Sorry I missed your comment. I'll give it a go. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r49891184
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
Let me try again, but when I was running the unit tests it gives me a NPE
for some reason, and been a while since I ran this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-168781103 @andrewor14 @dragos I think we figured out the testing problem at this point, I've tested this locally myself so @dragos if you like to try it out let me know, otherwise we should definitely merge this fix soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-164580763 @tnachen can you provide some guidance? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-163901420 I'd love to move this forward, but I need to know how you tested this (and replicate). ping @tnachen --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r47407104
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
also, what's the motivation for using java list instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r47406920
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
@tnachen I don't understand, `conf.getOption(...).toList` returns `Nil`,
but you can append to an empty list can't you?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-161336155 So, I like the code as it is now. I'm having trouble running Spark on Mesos in cluster mode, as always, so I didn't manage to test it, but that's most likely unrelated to code changes here. How did you test it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-160075491 @andrewor14 PTAL, it should be ready. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-156846731 **[Test build #45958 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45958/consoleFull)** for PR 8872 at commit [`7a16052`](https://github.com/apache/spark/commit/7a16052478d6f2723c57e21cb29748bce7bf25e0). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-156858796 **[Test build #45958 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45958/consoleFull)** for PR 8872 at commit [`7a16052`](https://github.com/apache/spark/commit/7a16052478d6f2723c57e21cb29748bce7bf25e0). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-156858834 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45958/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-156858833 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152947042 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152946954 **[Test build #44796 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44796/consoleFull)** for PR 8872 at commit [`cb876cf`](https://github.com/apache/spark/commit/cb876cf2f2486db6b63f167c41ddd113fd5b56b7). * This patch passes all tests. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_:\n * `case class Corr(`\n * `case class Corr(left: Expression, right: Expression)`\n * `case class RepartitionByExpression(`\n --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152947044 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44796/ Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r43587599
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
It was causing a null exception as the toList returns a Nil and with ++ .
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152918280 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152918579 **[Test build #44794 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44794/consoleFull)** for PR 8872 at commit [`ee6ad85`](https://github.com/apache/spark/commit/ee6ad85e04b3a7b0c7c30b19072ab311f4558205). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152918251 @andrewor14 @dragos all comments should be addressed now --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152918784 **[Test build #44794 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44794/consoleFull)** for PR 8872 at commit [`ee6ad85`](https://github.com/apache/spark/commit/ee6ad85e04b3a7b0c7c30b19072ab311f4558205). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_:\n * `case class Corr(`\n * `case class Corr(left: Expression, right: Expression)`\n * `case class RepartitionByExpression(`\n --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152922261 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152922265 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152922644 **[Test build #44796 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44796/consoleFull)** for PR 8872 at commit [`cb876cf`](https://github.com/apache/spark/commit/cb876cf2f2486db6b63f167c41ddd113fd5b56b7). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user andrewor14 commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152151363 @tnachen can you address the comments? I would like to get this merged. Also it's still failing style tests. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user BrickXu commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-152114579 @tnachen thanks very much, I will merge this PR in our private spark repo, and report issues ASAP if it does not work well. Thanks again! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r42725306
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -443,9 +444,13 @@ private[spark] class MesosClusterScheduler(
options
}
- private class ResourceOffer(val offer: Offer, var cpu: Double, var mem:
Double) {
+ private class ResourceOffer(
+ val offerId: OfferID,
+ val slaveId: SlaveID,
+ var resources: JList[Resource],
+ var used: Boolean) {
--- End diff --
I'd avoid mutable fields for little classes like this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r42724448
--- Diff:
core/src/test/scala/org/apache/spark/scheduler/mesos/MesosClusterSchedulerSuite.scala
---
@@ -72,4 +78,56 @@ class MesosClusterSchedulerSuite extends SparkFunSuite
with LocalSparkContext wi
val state = scheduler.getSchedulerState()
assert(state.queuedDrivers.isEmpty)
}
+
+ test("can handle multiple roles") {
+val conf = new SparkConf()
+conf.setMaster("mesos://localhost:5050")
+conf.setAppName("spark mesos")
+val scheduler = new MesosClusterScheduler(
+ new BlackHoleMesosClusterPersistenceEngineFactory, conf) {
+ override def start(): Unit = { ready = true }
+}
+scheduler.start()
+val driver = mock[SchedulerDriver]
+val response = scheduler.submitDriver(
+ new MesosDriverDescription("d1", "jar", 1500, 1, true,
+command,
+Map(("spark.mesos.executor.home", "test"), ("spark.app.name",
"test")),
+"s1",
+new Date()))
+assert(response.success)
+val offer = Offer.newBuilder()
+ .addResources(
+Resource.newBuilder().setRole("*")
+
.setScalar(Scalar.newBuilder().setValue(1).build()).setName("cpus").setType(Type.SCALAR))
+ .addResources(
+Resource.newBuilder().setRole("*")
+
.setScalar(Scalar.newBuilder().setValue(1000).build()).setName("mem").setType(Type.SCALAR))
+ .addResources(
+Resource.newBuilder().setRole("role2")
+
.setScalar(Scalar.newBuilder().setValue(1).build()).setName("cpus").setType(Type.SCALAR))
+ .addResources(
+Resource.newBuilder().setRole("role2")
+
.setScalar(Scalar.newBuilder().setValue(500).build()).setName("mem").setType(Type.SCALAR))
+ .setId(OfferID.newBuilder().setValue("o1").build())
+ .setFrameworkId(FrameworkID.newBuilder().setValue("f1").build())
+ .setSlaveId(SlaveID.newBuilder().setValue("s1").build())
+ .setHostname("host1")
+ .build()
+
+val capture = ArgumentCaptor.forClass(classOf[Collection[TaskInfo]])
+
+when(
+ driver.launchTasks(
+Matchers.eq(Collections.singleton(offer.getId)),
+capture.capture())
+).thenReturn(Status.valueOf(1))
+
+scheduler.resourceOffers(driver, List(offer).asJava)
+
+verify(driver, times(1)).launchTasks(
--- End diff --
Can you also test that the role is set? In other words, would this test
fail without this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r42724611
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -358,9 +358,10 @@ private[spark] class MesosClusterScheduler(
val appJar = CommandInfo.URI.newBuilder()
.setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
val builder = CommandInfo.newBuilder().addUris(appJar)
-val entries =
- (conf.getOption("spark.executor.extraLibraryPath").toList ++
-desc.command.libraryPathEntries)
+val entries = conf.getOption("spark.executor.extraLibraryPath")
+ .map(path => Seq(path) ++ desc.command.libraryPathEntries)
+ .getOrElse(desc.command.libraryPathEntries)
--- End diff --
This seems more complicated than the original version. Any particular
reason why you refactored it this way?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r42725607
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -457,34 +462,37 @@ private[spark] class MesosClusterScheduler(
private def scheduleTasks(
candidates: Seq[MesosDriverDescription],
afterLaunchCallback: (String) => Boolean,
- currentOffers: List[ResourceOffer],
- tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]): Unit = {
+ currentOffers: JList[ResourceOffer],
+ tasks: mutable.HashMap[OfferID, ArrayBuffer[TaskInfo]]):
JList[ResourceOffer] = {
for (submission <- candidates) {
val driverCpu = submission.cores
val driverMem = submission.mem
logTrace(s"Finding offer to launch driver with cpu: $driverCpu, mem:
$driverMem")
- val offerOption = currentOffers.find { o =>
-o.cpu >= driverCpu && o.mem >= driverMem
+ val offerOption = currentOffers.asScala.find { o =>
+getResource(o.resources, "cpus") >= driverCpu &&
+getResource(o.resources, "mem") >= driverMem
}
if (offerOption.isEmpty) {
logDebug(s"Unable to find offer to launch driver id:
${submission.submissionId}, " +
s"cpu: $driverCpu, mem: $driverMem")
} else {
val offer = offerOption.get
-offer.cpu -= driverCpu
-offer.mem -= driverMem
+offer.used = true
--- End diff --
What if the offer was already `used` here? One more reason to get rid of
the `used` field altogether. Make this method assume `currentOffers` are all
for the taking (as it does now), and returns all the remaining offers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r42725390
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -497,28 +505,28 @@ private[spark] class MesosClusterScheduler(
container, image, volumes = volumes, portmaps = portmaps)
taskInfo.setContainer(container.build())
}
-val queuedTasks = tasks.getOrElseUpdate(offer.offer.getId, new
ArrayBuffer[TaskInfo])
+val queuedTasks = tasks.getOrElseUpdate(offer.offerId, new
ArrayBuffer[TaskInfo])
queuedTasks += taskInfo.build()
-logTrace(s"Using offer ${offer.offer.getId.getValue} to launch
driver " +
+logTrace(s"Using offer ${offer.offerId.getValue} to launch driver
" +
submission.submissionId)
-val newState = new MesosClusterSubmissionState(submission, taskId,
offer.offer.getSlaveId,
+val newState = new MesosClusterSubmissionState(submission, taskId,
offer.slaveId,
None, new Date(), None)
launchedDrivers(submission.submissionId) = newState
launchedDriversState.persist(submission.submissionId, newState)
afterLaunchCallback(submission.submissionId)
}
}
+currentOffers
--- End diff --
It seems wrong to return all original offers. Why not filter out the `used`
ones? Callers of this method already do that, so you could get rid of the
`used` field and simply filter out here. It will simplify reasoning everywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147898639 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147898616 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user tnachen commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147899923 @dragos added unit test and fixed desc and comments now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900140 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43693/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900138 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900088 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900098 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900293 [Test build #43695 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43695/consoleFull) for PR 8872 at commit [`44b2ad4`](https://github.com/apache/spark/commit/44b2ad44d12c87f66c1dbd70a6672b7bb8710e12). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900555 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-147900558 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43695/ Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/8872#issuecomment-147900552
[Test build #43695 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43695/console)
for PR 8872 at commit
[`44b2ad4`](https://github.com/apache/spark/commit/44b2ad44d12c87f66c1dbd70a6672b7bb8710e12).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `def getPath = path.getOrElse(sys.error("Constructors must start at
a class type"))`
* `case class ExprId(id: Long, jvmId: UUID)`
* `case class WrapOption(optionType: DataType, child: Expression)`
* `class GenericArrayData(val array: Array[Any]) extends ArrayData `
* `trait QueryExecutionListener `
* `class ExecutionListenerManager extends Logging `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on the pull request: https://github.com/apache/spark/pull/8872#issuecomment-145809075 @tnachen this looks ok, but we need a better description. It's not clear what exactly is the problem, so not sure how to manually test it. Also, please add a unit test. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r40827123
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosSchedulerUtils.scala
---
@@ -189,7 +189,7 @@ private[mesos] trait MesosSchedulerUtils extends
Logging {
val filteredResources =
remainingResources.filter(r => r.getType != Value.Type.SCALAR ||
r.getScalar.getValue > 0.0)
-(filteredResources.toList, requestedResources.toList)
+(filteredResources.toList.asJava, requestedResources.toList.asJava)
--- End diff --
You don't need `toList.asJava`. `asJava` is enough, and saves a round of
copying.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-10749][MESOS] Support multiple roles wi...
Github user dragos commented on a diff in the pull request:
https://github.com/apache/spark/pull/8872#discussion_r40827416
--- Diff:
core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
---
@@ -542,9 +549,7 @@ private[spark] class MesosClusterScheduler(
tasks.foreach { case (offerId, taskInfos) =>
driver.launchTasks(Collections.singleton(offerId), taskInfos.asJava)
}
-offers.asScala
- .filter(o => !tasks.keySet.contains(o.getId))
- .foreach(o => driver.declineOffer(o.getId))
+currentOffers.asScala.filter(!_.used).foreach(o =>
driver.declineOffer(o.offerId))
--- End diff --
I think a for-comprehension would be slightly more efficient, though it's
not a big deal.
```
for { o <- currentOffers.asScala if !o.used } driver.declineOffer(o.offerId)
```
`filter` creates an intermediate collection, while inside a for this can be
fused into only one traversal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
