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

    https://github.com/apache/spark/pull/5722#discussion_r29690367
  
    --- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
    @@ -1959,30 +1959,41 @@ private[spark] object Utils extends Logging {
        * Attempt to start a service on the given port, or fail after a number 
of attempts.
        * Each subsequent attempt uses 1 + the port used in the previous 
attempt (unless the port is 0).
        *
    -   * @param startPort The initial port to start the service on.
    +   * @param port The minimum and maximum port to start the service on, 
separated by colon.
    +   *             If just set one number, take it as the minimum.
        * @param startService Function to start service on a given port.
        *                     This is expected to throw java.net.BindException 
on port collision.
        * @param conf A SparkConf used to get the maximum number of retries 
when binding to a port.
        * @param serviceName Name of the service.
        */
       def startServiceOnPort[T](
    -      startPort: Int,
    +      port: String,
           startService: Int => (T, Int),
           conf: SparkConf,
           serviceName: String = ""): (T, Int) = {
    -
    -    require(startPort == 0 || (1024 <= startPort && startPort < 65536),
    -      "startPort should be between 1024 and 65535 (inclusive), or 0 for a 
random free port.")
    +    val maxRetries = portMaxRetries(conf)
    +    val ports = port.split(":", 2)
    +    val (minPort, maxPort) = if (ports.length == 2) {
    +      (ports(0).toInt, ports(1).toInt)
    +    } else {
    +      val _minPort = ports(0).toInt
    +      (_minPort, math.min(65535, _minPort + maxRetries))
    +    }
    +    require(minPort == 0 || (1024 <= minPort && minPort <= 65535),
    +      s"Minimum port ${minPort} should be between 1024 and 65535 
(inclusive)," +
    +        " or 0 for a random free port.")
    +    require((1024 <= maxPort && maxPort <= 65535),
    +      s"Maximum port ${maxPort} should be between 1024 and 65535 
(inclusive).")
    +    require(minPort <= maxPort, s"Minimum ${minPort} port should not be" +
    +      s" less than the maximum ${maxPort}.")
     
         val serviceString = if (serviceName.isEmpty) "" else s" '$serviceName'"
    -    val maxRetries = portMaxRetries(conf)
         for (offset <- 0 to maxRetries) {
    --- End diff --
    
    It's a good question how to reconcile this with current behavior, yes. 
Right now, everything is a single port and implicitly allowed to try many 
different ports, up to a number of retries. After this, a single port will mean 
a single port only. I don't think both can be supported at the same time.
    
    The main issue I foresee is that port collisions on default ports will no 
longer be resolved automatically. I suppose that, where a user has set 
something to an explicit port, he/she probably doesn't necessarily want or 
expect different ports to get used. The user that has configured nothing may be 
surprised that starting two Spark drivers on one machine won't work anymore 
though.
    
    The various defaults in Spark (e.g. web UI on 4040) might change to 
"[4040,4055]" to match current behavior. It would be more explicit too. But it 
makes Spark and users hard-code this arbitrary "max retries" in the ranges. 
    
    Possibly there can be a new syntax like "[4040,]" meaning "try ports from 
4040". And then `maxRetries` can have its current role as controlling just how 
many more ports to try.


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