You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ankurcha <gi...@git.apache.org> on 2015/04/17 23:10:08 UTC

[GitHub] spark pull request: [SPARK-6707] - Mesos Scheduler should allow th...

GitHub user ankurcha opened a pull request:

    https://github.com/apache/spark/pull/5563

    [SPARK-6707] - Mesos Scheduler should allow the user to specify constraints based on slave attributes

    Currently, the mesos scheduler only looks at the 'cpu' and 'mem' resources when trying to determine the usablility of a resource offer from a mesos slave node. It may be preferable for the user to be able to ensure that the spark jobs are only started on a certain set of nodes (based on attributes). 
    
    For example, If the user sets a property, let's say `spark.mesos.constraints` is set to `tachyon=true;us-east-1=false`, then the resource offers will be checked to see if they meet both these constraints and only then will be accepted to start new executors.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ankurcha/spark mesos_attribs

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/5563.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5563
    
----
commit 608005f8172d95834eff726646d2dc734bcb152d
Author: Ankur Chauhan <ac...@brightcove.com>
Date:   2015-04-17T19:27:48Z

    First pass at add attributes filtering of offers to mesos scheduler

commit 8895eabd71b876930ecccd92ed7b76e36fb2d3c0
Author: Ankur Chauhan <ac...@brightcove.com>
Date:   2015-04-17T19:38:15Z

    code style fixes

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94953913
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30698/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by ankurcha <gi...@git.apache.org>.
Github user ankurcha commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28826649
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    +
    +  /** Helper method to get the key,value-set pair for a Mesos Attribute protobuf */
    +  private[mesos] def getAttribute(attr: Attribute) =
    +    (attr.getName, attr.getText.getValue.split(',').toSet)
    +
    +  /** Helper function to pull out a resource from a Mesos Resources protobuf */
    +  private[mesos] def getResource(res: JList[Resource], name: String): Double = {
    +    for (r <- res if r.getName == name) {
    +      return r.getScalar.getValue
    +    }
    +    0
    +  }
    +
    +  /** Build a Mesos resource protobuf object */
    +  private[mesos] def createResource(resourceName: String, quantity: Double): Protos.Resource = {
    +    Resource.newBuilder()
    +      .setName(resourceName)
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Value.Scalar.newBuilder().setValue(quantity).build())
    +      .build()
    +  }
    +
    +
    +  /**
    +   * Match the requirements (if any) to the offer's attributes.
    +   * if attribute requirements are not specified - return true
    +   * else if attribute is defined and no values are given, simple attribute presence is preformed
    +   * else if attribute name and value is specified, subset match is performed on slave attributes
    +   */
    +  private[mesos] def matchesAttributeRequirements(slaveOfferConstraints: Map[String, Set[String]],
    +                                                  offerAttributes: Map[String, Set[String]]) =
    +    if (slaveOfferConstraints.isEmpty) {
    +      true
    +    } else {
    +      slaveOfferConstraints.forall {
    +        // offer has the required attribute and subsumes the required values for that attribute
    +        case (name, requiredValues) =>
    +          offerAttributes.contains(name) &&
    +            (requiredValues.size == 0 ||
    +              (requiredValues forall ((v) => offerAttributes(name).contains(v))))
    --- End diff --
    
    I added a comment about the behaviour.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94967114
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30693/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-95994075
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30944/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28816152
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    +
    +  /** Helper method to get the key,value-set pair for a Mesos Attribute protobuf */
    +  private[mesos] def getAttribute(attr: Attribute) =
    +    (attr.getName, attr.getText.getValue.split(',').toSet)
    +
    +  /** Helper function to pull out a resource from a Mesos Resources protobuf */
    +  private[mesos] def getResource(res: JList[Resource], name: String): Double = {
    +    for (r <- res if r.getName == name) {
    +      return r.getScalar.getValue
    +    }
    +    0
    +  }
    +
    +  /** Build a Mesos resource protobuf object */
    +  private[mesos] def createResource(resourceName: String, quantity: Double): Protos.Resource = {
    +    Resource.newBuilder()
    +      .setName(resourceName)
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Value.Scalar.newBuilder().setValue(quantity).build())
    +      .build()
    +  }
    +
    +
    +  /**
    +   * Match the requirements (if any) to the offer's attributes.
    +   * if attribute requirements are not specified - return true
    +   * else if attribute is defined and no values are given, simple attribute presence is preformed
    +   * else if attribute name and value is specified, subset match is performed on slave attributes
    +   */
    +  private[mesos] def matchesAttributeRequirements(slaveOfferConstraints: Map[String, Set[String]],
    +                                                  offerAttributes: Map[String, Set[String]]) =
    +    if (slaveOfferConstraints.isEmpty) {
    +      true
    +    } else {
    +      slaveOfferConstraints.forall {
    +        // offer has the required attribute and subsumes the required values for that attribute
    +        case (name, requiredValues) =>
    +          offerAttributes.contains(name) &&
    +            (requiredValues.size == 0 ||
    +              (requiredValues forall ((v) => offerAttributes(name).contains(v))))
    --- End diff --
    
    Again I think we always use dots with methods.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28819164
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    --- End diff --
    
    :) i see, do you recommend using it like this? every package.scala seems to be just defining an object


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-95994074
  
      [Test build #30944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30944/consoleFull) for   PR 5563 at commit [`a039c08`](https://github.com/apache/spark/commit/a039c081ab8626c75500d9c3960072d7eab5b9d0).
     * This patch **fails RAT tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `final class IDF extends Estimator[IDFModel] with IDFBase `
    
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94934927
  
    Jenkins, this is ok to test, but we will need to rebase this to master to resolve the merge conflicts.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by ankurcha <gi...@git.apache.org>.
Github user ankurcha commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94941042
  
    Thanks all, I'll make the changes and rebase the pull request. This is my first foray into the world of any "real" scala so really appreciate the feedback.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by ankurcha <gi...@git.apache.org>.
Github user ankurcha commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28825617
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    +
    +  /** Helper method to get the key,value-set pair for a Mesos Attribute protobuf */
    +  private[mesos] def getAttribute(attr: Attribute) =
    +    (attr.getName, attr.getText.getValue.split(',').toSet)
    +
    +  /** Helper function to pull out a resource from a Mesos Resources protobuf */
    +  private[mesos] def getResource(res: JList[Resource], name: String): Double = {
    +    for (r <- res if r.getName == name) {
    +      return r.getScalar.getValue
    +    }
    +    0
    +  }
    +
    +  /** Build a Mesos resource protobuf object */
    +  private[mesos] def createResource(resourceName: String, quantity: Double): Protos.Resource = {
    +    Resource.newBuilder()
    +      .setName(resourceName)
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Value.Scalar.newBuilder().setValue(quantity).build())
    +      .build()
    +  }
    +
    +
    +  /**
    +   * Match the requirements (if any) to the offer's attributes.
    +   * if attribute requirements are not specified - return true
    +   * else if attribute is defined and no values are given, simple attribute presence is preformed
    +   * else if attribute name and value is specified, subset match is performed on slave attributes
    +   */
    +  private[mesos] def matchesAttributeRequirements(slaveOfferConstraints: Map[String, Set[String]],
    --- End diff --
    
    Moved them to new line.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-96001957
  
      [Test build #30945 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30945/consoleFull) for   PR 5563 at commit [`b344392`](https://github.com/apache/spark/commit/b344392a8c609c91198e7727948feaf2a7000a9f).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] - Mesos Scheduler should allow th...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94076467
  
    Can one of the admins verify 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94967098
  
      [Test build #30693 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30693/consoleFull) for   PR 5563 at commit [`8895eab`](https://github.com/apache/spark/commit/8895eabd71b876930ecccd92ed7b76e36fb2d3c0).
     * This patch **passes all tests**.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.
     * This patch **adds the following new dependencies:**
       * `commons-math3-3.1.1.jar`
       * `snappy-java-1.1.1.6.jar`
    
     * This patch **removes the following dependencies:**
       * `commons-math3-3.4.1.jar`
       * `snappy-java-1.1.1.7.jar`



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28816045
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    --- End diff --
    
    I don't ever see usage of a package object in Spark, not sure we'd like to set a precedent here.
    @andrewor14 is more familiar with the style I'll let him comment on this, but I'll recommend not doing 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94955424
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30699/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-95994064
  
      [Test build #30944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30944/consoleFull) for   PR 5563 at commit [`a039c08`](https://github.com/apache/spark/commit/a039c081ab8626c75500d9c3960072d7eab5b9d0).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94921561
  
    Btw why only apply constraints on fine grain mode? Why not coarse grain?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94955650
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30703/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by ankurcha <gi...@git.apache.org>.
Github user ankurcha commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94954323
  
    @tnachen - I added support to the coarse scheduler too. I had missed that 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94955646
  
      [Test build #30703 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30703/consoleFull) for   PR 5563 at commit [`24e4793`](https://github.com/apache/spark/commit/24e4793fba75bd4afae52bbaecfc5bf593754eab).
     * This patch **fails RAT tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94956237
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30701/
    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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28816208
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    +
    +  /** Helper method to get the key,value-set pair for a Mesos Attribute protobuf */
    +  private[mesos] def getAttribute(attr: Attribute) =
    +    (attr.getName, attr.getText.getValue.split(',').toSet)
    +
    +  /** Helper function to pull out a resource from a Mesos Resources protobuf */
    +  private[mesos] def getResource(res: JList[Resource], name: String): Double = {
    +    for (r <- res if r.getName == name) {
    +      return r.getScalar.getValue
    +    }
    +    0
    +  }
    +
    +  /** Build a Mesos resource protobuf object */
    +  private[mesos] def createResource(resourceName: String, quantity: Double): Protos.Resource = {
    +    Resource.newBuilder()
    +      .setName(resourceName)
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Value.Scalar.newBuilder().setValue(quantity).build())
    +      .build()
    +  }
    +
    +
    +  /**
    +   * Match the requirements (if any) to the offer's attributes.
    +   * if attribute requirements are not specified - return true
    +   * else if attribute is defined and no values are given, simple attribute presence is preformed
    +   * else if attribute name and value is specified, subset match is performed on slave attributes
    +   */
    +  private[mesos] def matchesAttributeRequirements(slaveOfferConstraints: Map[String, Set[String]],
    +                                                  offerAttributes: Map[String, Set[String]]) =
    +    if (slaveOfferConstraints.isEmpty) {
    +      true
    +    } else {
    +      slaveOfferConstraints.forall {
    +        // offer has the required attribute and subsumes the required values for that attribute
    +        case (name, requiredValues) =>
    +          offerAttributes.contains(name) &&
    +            (requiredValues.size == 0 ||
    +              (requiredValues forall ((v) => offerAttributes(name).contains(v))))
    --- End diff --
    
    Also I'm wondering about case sensitiveness here, it seems to assume we care about case. I'll add a comment in general about 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by ankurcha <gi...@git.apache.org>.
Github user ankurcha commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28825595
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    --- End diff --
    
    Added `MesosUtils`. I am not a big fan of this name so if you have a better one, please let me know.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94955632
  
      [Test build #30703 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30703/consoleFull) for   PR 5563 at commit [`24e4793`](https://github.com/apache/spark/commit/24e4793fba75bd4afae52bbaecfc5bf593754eab).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by tnachen <gi...@git.apache.org>.
Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28816099
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    +
    +  /** Helper method to get the key,value-set pair for a Mesos Attribute protobuf */
    +  private[mesos] def getAttribute(attr: Attribute) =
    +    (attr.getName, attr.getText.getValue.split(',').toSet)
    +
    +  /** Helper function to pull out a resource from a Mesos Resources protobuf */
    +  private[mesos] def getResource(res: JList[Resource], name: String): Double = {
    +    for (r <- res if r.getName == name) {
    +      return r.getScalar.getValue
    +    }
    +    0
    +  }
    +
    +  /** Build a Mesos resource protobuf object */
    +  private[mesos] def createResource(resourceName: String, quantity: Double): Protos.Resource = {
    +    Resource.newBuilder()
    +      .setName(resourceName)
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Value.Scalar.newBuilder().setValue(quantity).build())
    +      .build()
    +  }
    +
    +
    +  /**
    +   * Match the requirements (if any) to the offer's attributes.
    +   * if attribute requirements are not specified - return true
    +   * else if attribute is defined and no values are given, simple attribute presence is preformed
    +   * else if attribute name and value is specified, subset match is performed on slave attributes
    +   */
    +  private[mesos] def matchesAttributeRequirements(slaveOfferConstraints: Map[String, Set[String]],
    --- End diff --
    
    Spark style usually moves this to the next line with 4 space indents


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28818950
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    --- End diff --
    
    we do have package objects :)
    https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/package.scala


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/5563#discussion_r28820124
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/package.scala ---
    @@ -0,0 +1,96 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster
    +
    +import java.util.{List => JList}
    +
    +import com.google.common.base.Splitter
    +import org.apache.mesos.Protos
    +import org.apache.mesos.Protos._
    +
    +import scala.collection.JavaConversions._
    +
    +
    +package object mesos {
    --- End diff --
    
    Yeah that's a good point. I think the convention elsewhere is that we define a `XUtils` object and do `XUtils.methodName()` for common methods (see `Utils`, `JettyUtils`, `AkkaUtils` etc.). It might make more sense to do that here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-94935974
  
      [Test build #30693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30693/consoleFull) for   PR 5563 at commit [`8895eab`](https://github.com/apache/spark/commit/8895eabd71b876930ecccd92ed7b76e36fb2d3c0).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-6707] [core]: Mesos Scheduler should al...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/5563#issuecomment-96766967
  
    Can one of the admins verify 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org