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

    https://github.com/apache/spark/pull/15237#discussion_r94501620
  
    --- Diff: 
core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala ---
    @@ -102,38 +103,52 @@ private[spark] class FairSchedulableBuilder(val 
rootPool: Pool, conf: SparkConf)
         for (poolNode <- (xml \\ POOLS_PROPERTY)) {
     
           val poolName = (poolNode \ POOL_NAME_PROPERTY).text
    -      var schedulingMode = DEFAULT_SCHEDULING_MODE
    -      var minShare = DEFAULT_MINIMUM_SHARE
    -      var weight = DEFAULT_WEIGHT
    -
    -      val xmlSchedulingMode = (poolNode \ SCHEDULING_MODE_PROPERTY).text
    -      if (xmlSchedulingMode != "") {
    -        try {
    -          schedulingMode = SchedulingMode.withName(xmlSchedulingMode)
    -        } catch {
    -          case e: NoSuchElementException =>
    -            logWarning(s"Unsupported schedulingMode: $xmlSchedulingMode, " 
+
    -              s"using the default schedulingMode: $schedulingMode")
    -        }
    -      }
     
    -      val xmlMinShare = (poolNode \ MINIMUM_SHARES_PROPERTY).text
    -      if (xmlMinShare != "") {
    -        minShare = xmlMinShare.toInt
    -      }
    +      val xmlSchedulingMode = (poolNode \ 
SCHEDULING_MODE_PROPERTY).text.trim.toUpperCase
    +      val schedulingMode = getSchedulingModeValue(xmlSchedulingMode, 
DEFAULT_SCHEDULING_MODE)
     
    -      val xmlWeight = (poolNode \ WEIGHT_PROPERTY).text
    -      if (xmlWeight != "") {
    -        weight = xmlWeight.toInt
    -      }
    +      val xmlMinShare = (poolNode \ MINIMUM_SHARES_PROPERTY).text.trim
    +      val minShare = getIntValue(MINIMUM_SHARES_PROPERTY, xmlMinShare, 
DEFAULT_MINIMUM_SHARE)
    +
    +      val xmlWeight = (poolNode \ WEIGHT_PROPERTY).text.trim
    +      val weight = getIntValue(WEIGHT_PROPERTY, xmlWeight, DEFAULT_WEIGHT)
    +
    +      rootPool.addSchedulable(new Pool(poolName, schedulingMode, minShare, 
weight))
     
    -      val pool = new Pool(poolName, schedulingMode, minShare, weight)
    -      rootPool.addSchedulable(pool)
           logInfo("Created pool %s, schedulingMode: %s, minShare: %d, weight: 
%d".format(
             poolName, schedulingMode, minShare, weight))
         }
       }
     
    +  private def getSchedulingModeValue(data: String, defaultValue: 
SchedulingMode): SchedulingMode = {
    +    val warningMessage = s"Unsupported schedulingMode: $data, using the 
default schedulingMode: " +
    +      s"$defaultValue"
    +    try {
    +      if (SchedulingMode.withName(data) != SchedulingMode.NONE) {
    +        SchedulingMode.withName(data)
    +      } else {
    +        logWarning(warningMessage)
    +        defaultValue
    +      }
    +    } catch {
    +      case e: NoSuchElementException =>
    +        logWarning(warningMessage)
    +        defaultValue
    +    }
    +  }
    +
    +  private def getIntValue(propertyName: String, data: String, 
defaultValue: Int): Int = {
    +    try {
    +      data.toInt
    +    } catch {
    +      case e: NumberFormatException =>
    +        logWarning(s"Error while loading scheduler allocation file at 
$schedulerAllocFile. " +
    --- End diff --
    
    ugh, this is kind of a nuisance, but I realize now that 
`schedulerAllocFile` isn't necessarily the right file -- that might be empty, 
and there might be a `fairscheduler.xml` sitting on the classpath.  Can you get 
the right file name in both cases?  (Better to leave it to not include any 
filename, than to 
    
    And can the warning include the poolname as well?
    
    Finally, it would be nice to add this extra info to the mode warning too.


---
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