-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-11714][Mesos] Make Spark on Mesos honor port restrictions on coarse grain mode #11157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
53f2ced
a4e575d
c6ceb8b
e9f37bc
5955973
0432771
dba3e34
47d6a9f
45b7984
84b3ca9
8b61dd2
2493d2a
4bdb859
6f7f2d3
af9b19b
b46b9d4
3bc31cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -186,6 +186,11 @@ private[spark] class CoarseMesosSchedulerBackend( | |
| .setValue(value) | ||
| .build()) | ||
| } | ||
|
|
||
| environment.addVariables(Environment.Variable.newBuilder() | ||
| .setName("AVAILABLE_PORTS") | ||
| .setValue(getRangeResource(offer.getResourcesList, "ports").mkString(" "))) | ||
|
|
||
| val command = CommandInfo.newBuilder() | ||
| .setEnvironment(environment) | ||
|
|
||
|
|
@@ -376,8 +381,10 @@ private[spark] class CoarseMesosSchedulerBackend( | |
|
|
||
| val (afterCPUResources, cpuResourcesToUse) = | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you factor this all out into a getResources() method?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
| partitionResources(resources, "cpus", taskCPUs) | ||
| val (resourcesLeft, memResourcesToUse) = | ||
| val (remainingMemResources, memResourcesToUse) = | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/remainingMemResources/afterMemResources to be consistent w/
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i will fix that. |
||
| partitionResources(afterCPUResources.asJava, "mem", taskMemory) | ||
| val (resourcesLeft, portResourcesToUse) = remainingMemResources | ||
| .partition {r => ! ( r.getType == Value.Type.RANGES & r.getName == "ports" )} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is still style problems here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes havent finished it.... port resources need to be managed as cpus eg. keep global logistics... but what exactly is the problem for you? |
||
|
|
||
| val taskBuilder = MesosTaskInfo.newBuilder() | ||
| .setTaskId(TaskID.newBuilder().setValue(taskId.toString).build()) | ||
|
|
@@ -386,6 +393,7 @@ private[spark] class CoarseMesosSchedulerBackend( | |
| .setName("Task " + taskId) | ||
| .addAllResources(cpuResourcesToUse.asJava) | ||
| .addAllResources(memResourcesToUse.asJava) | ||
| .addAllResources(portResourcesToUse.asJava) | ||
|
|
||
| sc.conf.getOption("spark.mesos.executor.docker.image").foreach { image => | ||
| MesosSchedulerBackendUtil | ||
|
|
@@ -407,13 +415,16 @@ private[spark] class CoarseMesosSchedulerBackend( | |
| val offerCPUs = getResource(resources, "cpus").toInt | ||
| val cpus = executorCores(offerCPUs) | ||
| val mem = executorMemory(sc) | ||
| val ports = getRangeResource(resources, "ports") | ||
| val meetsPortRequirements = checkPorts(sc, ports) | ||
|
|
||
| cpus > 0 && | ||
| cpus <= offerCPUs && | ||
| cpus + totalCoresAcquired <= maxCores && | ||
| mem <= offerMem && | ||
| numExecutors() < executorLimit && | ||
| slaves.get(slaveId).map(_.taskFailures).getOrElse(0) < MAX_SLAVE_FAILURES | ||
| slaves.get(slaveId).map(_.taskFailures).getOrElse(0) < MAX_SLAVE_FAILURES && | ||
| meetsPortRequirements | ||
| } | ||
|
|
||
| private def executorCores(offerCPUs: Int): Int = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,6 +146,19 @@ private[mesos] trait MesosSchedulerUtils extends Logging { | |
| res.asScala.filter(_.getName == name).map(_.getScalar.getValue).sum | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The additions to this file seem overly complex for just grabbing port resources. Maybe all this complexity is necessary, but I'm skeptical. I started in on it and got lost. I don't have time to review it all today. Please ensure there is no more room for simplification. |
||
| /** Transforms a range resource to a list of ranges | ||
| * | ||
| * @param res the mesos resource list | ||
| * @param name the name of the resource | ||
| * @return the list of ranges returned | ||
| */ | ||
| protected def getRangeResource(res: JList[Resource], name: String): List[(Long, Long)] = { | ||
| // A resource can have multiple values in the offer since it can either be from | ||
| // a specific role or wildcard. | ||
| res.asScala.filter(_.getName == name).flatMap(_.getRanges.getRangeList.asScala | ||
| .map(r => (r.getBegin, r.getEnd)).toList).toList | ||
| } | ||
|
|
||
| /** | ||
| * Signal that the scheduler has registered with Mesos. | ||
| */ | ||
|
|
@@ -353,4 +366,25 @@ private[mesos] trait MesosSchedulerUtils extends Logging { | |
| sc.conf.getTimeAsSeconds("spark.mesos.rejectOfferDurationForUnmetConstraints", "120s") | ||
| } | ||
|
|
||
| /** Checks executor ports if they are within some range of the offered list of ports ranges. | ||
| * | ||
| * @param sc the Spark Context | ||
| * @param ports the list of ports to check | ||
| * @return true if ports are within range false otherwise | ||
| */ | ||
| protected def checkPorts(sc: SparkContext, ports: List[(Long, Long)]): Boolean = { | ||
|
|
||
| def checkIfInRange(port: Int, ps: List[(Long, Long)]): Boolean = { | ||
| ps.exists(r => r._1 <= port & r._2 >= port) | ||
| } | ||
|
|
||
| val portsToCheck = List(sc.conf.getInt("spark.executor.port", 0), | ||
| sc.conf.getInt("spark.blockManager.port", 0)) | ||
| val nonZeroPorts = portsToCheck.filter(_ != 0) | ||
| val withinRange = nonZeroPorts.forall(p => checkIfInRange(p, ports)) | ||
|
|
||
| // make sure we have enough ports to allocate per offer | ||
| ports.map(r => r._2 - r._1 + 1).sum >= portsToCheck.size && withinRange | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1940,57 +1940,131 @@ 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 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. | ||
| * @return (service: T, port: Int) | ||
| */ | ||
| * 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). | ||
| * It takes into consideration port restrictions through the env var AVAILABLE_PORTS | ||
| * | ||
| * @param startPort The initial port to start the service on. | ||
| * @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. | ||
| * @return (service: T, port: Int) | ||
| */ | ||
| def startServiceOnPort[T]( | ||
| startPort: Int, | ||
| startService: Int => (T, Int), | ||
| conf: SparkConf, | ||
| serviceName: String = ""): (T, Int) = { | ||
| startPort: Int, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix indentation |
||
| startService: Int => (T, Int), | ||
| conf: SparkConf, | ||
| serviceName: String = ""): (T, Int) = { | ||
|
|
||
| val serviceString = if (serviceName.isEmpty) "" else s" '$serviceName'" | ||
|
|
||
| // define some helpers, they all share common data, maybe a service abstract class | ||
| // for all services could be a good fit here. | ||
|
|
||
| def portRangeToList(ranges: String): List[(Long, Long)] = { | ||
| ranges.split(" ").map { r => val ret = r.substring(1, r.length - 1).split(",") | ||
| (ret(0).toLong, ret(1).toLong) | ||
| }.toList | ||
| } | ||
|
|
||
| def startOnce(tryPort: Int): (Option[T], Int) = { | ||
| val serviceString = if (serviceName.isEmpty) "" else s" '$serviceName'" | ||
| try { | ||
| val (service, port) = startService(tryPort) | ||
| logInfo(s"Successfully started service$serviceString on port $port.") | ||
| (Some(service), port) | ||
| } catch { | ||
| case e: Exception if isBindCollision(e) => | ||
| logWarning(s"Service$serviceString could not bind on port $tryPort. ") | ||
| (None, -1) | ||
| } | ||
| } | ||
|
|
||
| def retryPort(next: Int => Int, maxRetries: Int): (T, Int) = { | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra space |
||
| for (offset <- 0 to maxRetries) { | ||
| val tryPort = next(offset) | ||
| try { | ||
| val (service, port) = startService(tryPort) | ||
| logInfo(s"Successfully started service$serviceString on port $port.") | ||
| return (service, port) | ||
| } catch { | ||
| case e: Exception if isBindCollision(e) => | ||
| if (offset >= maxRetries) { | ||
| val exceptionMessage = | ||
| s"${e.getMessage}: Service$serviceString failed after $maxRetries retries!" | ||
| val exception = new BindException(exceptionMessage) | ||
| // restore original stack trace | ||
| exception.setStackTrace(e.getStackTrace) | ||
| throw exception | ||
| } | ||
| logWarning(s"Service$serviceString could not bind on port $tryPort.") | ||
| } | ||
| } | ||
| // Should never happen | ||
| throw new SparkException(s"Failed to start service$serviceString on port $startPort") | ||
| } | ||
|
|
||
| def startFromAvailable(rand: Boolean = false): (T, Int) = { | ||
|
|
||
| val ports = portRangeToList(sys.env.get("AVAILABLE_PORTS").get) // checked above for empty | ||
|
|
||
| val filteredPorts = ports.map(r => (r._1 to r._2).toList).reduce(_ ++ _). | ||
| distinct.filterNot(_ == startPort) | ||
| val maxRetries = Math.min(portMaxRetries(conf), filteredPorts.size) | ||
|
|
||
| val availPorts = { | ||
| if (rand) { | ||
| scala.util.Random.shuffle(filteredPorts.take(maxRetries)).toArray | ||
| } else { | ||
| filteredPorts.sorted.toArray | ||
| } | ||
| } | ||
|
|
||
| val tryPort = (x: Int) => availPorts(x).toInt | ||
|
|
||
| retryPort(tryPort, maxRetries) | ||
| } | ||
|
|
||
| // main port selection logic | ||
| require(startPort == 0 || (1024 <= startPort && startPort < 65536), | ||
| "startPort should be between 1024 and 65535 (inclusive), or 0 for a random free port.") | ||
|
|
||
| val serviceString = if (serviceName.isEmpty) "" else s" '$serviceName'" | ||
| val maxRetries = portMaxRetries(conf) | ||
| for (offset <- 0 to maxRetries) { | ||
| if (sys.env.get("AVAILABLE_PORTS").isDefined) { // check if there are deployment restrictions | ||
|
|
||
| if (startPort != 0) { | ||
|
|
||
| val (service, port) = startOnce(startPort) // try once the provided specific port | ||
|
|
||
| if (port != -1) { | ||
| (service.get, port) // success | ||
| } else { | ||
| // try next available port one by one | ||
| startFromAvailable() | ||
| } | ||
| } else { | ||
| // try a random port from the available ones | ||
| startFromAvailable(true) | ||
| } | ||
|
|
||
| } else { // no restrictions, handled as usual, exactly the same path as in previous versions | ||
|
|
||
| val maxRetries = portMaxRetries(conf) | ||
|
|
||
| // Do not increment port if startPort is 0, which is treated as a special port | ||
| val tryPort = if (startPort == 0) { | ||
| val tryPort = (x: Int) => if (startPort == 0) { | ||
| startPort | ||
| } else { | ||
| // If the new port wraps around, do not try a privilege port | ||
| ((startPort + offset - 1024) % (65536 - 1024)) + 1024 | ||
| } | ||
| try { | ||
| val (service, port) = startService(tryPort) | ||
| logInfo(s"Successfully started service$serviceString on port $port.") | ||
| return (service, port) | ||
| } catch { | ||
| case e: Exception if isBindCollision(e) => | ||
| if (offset >= maxRetries) { | ||
| val exceptionMessage = | ||
| s"${e.getMessage}: Service$serviceString failed after $maxRetries retries!" | ||
| val exception = new BindException(exceptionMessage) | ||
| // restore original stack trace | ||
| exception.setStackTrace(e.getStackTrace) | ||
| throw exception | ||
| } | ||
| logWarning(s"Service$serviceString could not bind on port $tryPort. " + | ||
| s"Attempting port ${tryPort + 1}.") | ||
| ((startPort + x - 1024) % (65536 - 1024)) + 1024 | ||
| } | ||
|
|
||
| retryPort(tryPort, maxRetries) | ||
| } | ||
| // Should never happen | ||
| throw new SparkException(s"Failed to start service$serviceString on port $startPort") | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Return whether the exception is caused by an address-port collision when binding. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the executor isn't actually started here, can we instead print "setting spark.executor.port to {}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.