-
Notifications
You must be signed in to change notification settings - Fork 117
Allow setting memory on the driver submission server. #161
Changes from all commits
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 |
|---|---|---|
|
|
@@ -64,6 +64,19 @@ private[spark] class Client( | |
| .map(_.split(",")) | ||
| .getOrElse(Array.empty[String]) | ||
|
|
||
| // Memory settings | ||
| private val driverMemoryMb = sparkConf.get(org.apache.spark.internal.config.DRIVER_MEMORY) | ||
| private val driverSubmitServerMemoryMb = sparkConf.get(KUBERNETES_DRIVER_SUBMIT_SERVER_MEMORY) | ||
| private val driverSubmitServerMemoryString = sparkConf.get( | ||
| KUBERNETES_DRIVER_SUBMIT_SERVER_MEMORY.key, | ||
| KUBERNETES_DRIVER_SUBMIT_SERVER_MEMORY.defaultValueString) | ||
| private val driverContainerMemoryMb = driverMemoryMb + driverSubmitServerMemoryMb | ||
|
Member
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. Why do we add the two here? Shouldn't we ideally take the max(...) since only one of them is running at a time, either the submit server or the driver code?
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. Actually we don't shut down the submit server after it starts the application, so both will be running for the whole time.
Member
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. Saw @ash211 opened an issue. We should solve this in a better way after the alpha. |
||
| private val memoryOverheadMb = sparkConf | ||
| .get(KUBERNETES_DRIVER_MEMORY_OVERHEAD) | ||
| .getOrElse(math.max((MEMORY_OVERHEAD_FACTOR * driverContainerMemoryMb).toInt, | ||
| MEMORY_OVERHEAD_MIN)) | ||
| private val driverContainerMemoryWithOverhead = driverContainerMemoryMb + memoryOverheadMb | ||
|
|
||
| private val waitForAppCompletion: Boolean = sparkConf.get(WAIT_FOR_APP_COMPLETION) | ||
|
|
||
| private val secretBase64String = { | ||
|
|
@@ -408,6 +421,12 @@ private[spark] class Client( | |
| .withPath("/v1/submissions/ping") | ||
| .withNewPort(SUBMISSION_SERVER_PORT_NAME) | ||
| .build() | ||
| val driverMemoryQuantity = new QuantityBuilder(false) | ||
| .withAmount(s"${driverContainerMemoryMb}M") | ||
| .build() | ||
| val driverMemoryLimitQuantity = new QuantityBuilder(false) | ||
| .withAmount(s"${driverContainerMemoryWithOverhead}M") | ||
| .build() | ||
| val driverPod = kubernetesClient.pods().createNew() | ||
| .withNewMetadata() | ||
| .withName(kubernetesAppId) | ||
|
|
@@ -442,7 +461,16 @@ private[spark] class Client( | |
| .withName(ENV_SUBMISSION_SERVER_PORT) | ||
| .withValue(SUBMISSION_SERVER_PORT.toString) | ||
| .endEnv() | ||
| // Note that SPARK_DRIVER_MEMORY only affects the REST server via spark-class. | ||
| .addNewEnv() | ||
| .withName(ENV_DRIVER_MEMORY) | ||
| .withValue(driverSubmitServerMemoryString) | ||
| .endEnv() | ||
| .addToEnv(sslConfiguration.sslPodEnvVars: _*) | ||
| .withNewResources() | ||
| .addToRequests("memory", driverMemoryQuantity) | ||
| .addToLimits("memory", driverMemoryLimitQuantity) | ||
| .endResources() | ||
|
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. what were the defaults that we were relying on before?
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. There was no reliable default - probably just defaults to the underlying container runtime. |
||
| .withPorts(containerPorts.asJava) | ||
| .withNewReadinessProbe().withHttpGet(probePingHttpGet).endReadinessProbe() | ||
| .endContainer() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,7 @@ package object constants { | |
| private[spark] val ENV_EXECUTOR_MEMORY = "SPARK_EXECUTOR_MEMORY" | ||
| private[spark] val ENV_APPLICATION_ID = "SPARK_APPLICATION_ID" | ||
| private[spark] val ENV_EXECUTOR_ID = "SPARK_EXECUTOR_ID" | ||
| private[spark] val ENV_DRIVER_MEMORY = "SPARK_DRIVER_MEMORY" | ||
|
Member
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. Where is this environment variable used? I didn't see any corresponding change to the dockerfile launching the driver.
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. It's used in the |
||
|
|
||
| // Annotation keys | ||
| private[spark] val ANNOTATION_PROVIDE_EXTERNAL_URI = | ||
|
|
@@ -74,4 +75,6 @@ package object constants { | |
| private[spark] val DRIVER_CONTAINER_NAME = "spark-kubernetes-driver" | ||
| private[spark] val KUBERNETES_SUBMIT_SSL_NAMESPACE = "kubernetes.submit" | ||
| private[spark] val KUBERNETES_MASTER_INTERNAL_URL = "https://kubernetes.default.svc" | ||
| private[spark] val MEMORY_OVERHEAD_FACTOR = 0.10 | ||
| private[spark] val MEMORY_OVERHEAD_MIN = 384L | ||
| } | ||
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.
Why do we have separate
driverSubmitServerMemoryMbanddriverSubmitServerMemoryStringand similarly for executors?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.
The strings are used in the environment variables so that they can be passed directly to the JVMs launch commands.
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.
It seems like we could achieve this using
Utils.memoryStringToMb. Just a minor nit because it seems like repeated logic to fetch the same parameter in different ways.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.
The trouble there is when we use
SparkConf.get(config)it returns us a long, not a string - where the long is the number of megabytes pre-converted from the string value. I think the issue is that we want to go in the other direction; that is, to convert the numeric value we get fromSparkConf.getinto a memory string.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.
Hmm.. I wonder if the Utils package has a helper for that. But in any case, it isn't a major concern. Merging. Thanks!