Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/11129#discussion_r53272901
  
    --- Diff: 
yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
    @@ -307,8 +307,14 @@ private[yarn] class YarnAllocator(
           nodes: Array[String],
           racks: Array[String]): ContainerRequest = {
         nodeLabelConstructor.map { constructor =>
    +      val labelExp = if ((racks != null && (!racks.isEmpty))
    +        || (nodes != null && (!nodes.isEmpty))) {
    +        null
    +      } else {
    +        labelExpression.orNull
    +      }
           constructor.newInstance(resource, nodes, racks, RM_REQUEST_PRIORITY, 
true: java.lang.Boolean,
    -        labelExpression.orNull)
    +        labelExp)
    --- End diff --
    
    Normally if user set this label expression configuration, they want the 
label-based scheduling obviously. But in your implementation you silently 
disable this, this will make user confuse. Also since label-based scheduling 
cannot be worked with locality preferences in YARN side (I think it is 
intentionally), it would be better to ignore the locality information 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 [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to